You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Sheng Yang <sh...@yasker.org> on 2013/11/15 03:01:25 UTC

Re: Coverage Analysis: Few Issues

Hi Will,

Could you check on Palo Alto's duplicate api commands? They suppose to be
merged as one command I think.

BTW, how can this works? Did it broke SRX?

--Sheng


On Thu, Nov 14, 2013 at 2:55 AM, Santhosh Edukulla <
santhosh.edukulla@citrix.com> wrote:

> Team,
>
> While running code coverage analysis with sonar for integration tests,
> based upon the errors thrown, i could see the below issues\notes with CS
> project.
>
> Issue1:
>
> The coverage tool is complaining about duplicate sources  for below files.
> These are available with same name under folders
> ./cloudstack/plugins/network-elements/juniper-srx and as well under
> ..../palo-alto.
>
>
> ./cloudstack/plugins/network-elements/palo-alto/src/com/cloud/api/commands/AddExternalFirewallCmd.java
>
> ./cloudstack/plugins/network-elements/palo-alto/src/com/cloud/api/commands/DeleteExternalFirewallCmd.java
>
> ./cloudstack/plugins/network-elements/palo-alto/src/com/cloud/api/commands/ListExternalFirewallCmd.java
>
> I renamed one while running analysis to a different name.  It proceeded
> further with its analysis once renamed.
>
> Is it intentional to have same name or can be renamed?
>
> Issue2:
>
> The source directory does not correspond to package declaration for  code
> files under
>
> /root/softwares/cscode/cloudstack/services/console-proxy-rdp/rdpconsole/src/main/java/rdpclient/
>
> This error when compared to other files in the similar path has a
> different package convention and usage.
>
> /root/softwares/cscode/cloudstack/services/secondary-storage/src/org/apache/cloudstack/storage/
>
> Changing tool configuration properties worked to over come this, but Is it
> intentional to have a different package structure for rdpclient code base
> against others?
>
>
>
> Thanks!
> Santhosh
>

Re: Coverage Analysis: Few Issues

Posted by Will Stevens <ws...@cloudops.com>.
I will just remove them.  It appears the only reason they are there is for
backwards compatibility.  I have checked other plugins and they all just
expose the device specific api commands (Other than SRX which exposes both).

The problem with merging them into one command is the PaloAlto plugin is in
core and the SRX plugin is in 'noredist', so I don't really want to create
any dependency requirements on other plugins.

Cheers,

Will


On Fri, Nov 15, 2013 at 1:56 PM, Sheng Yang <sh...@yasker.org> wrote:

> Sure, if you don't need these addExternalFirewall etc., you can just
> remove them.
>
> Because the name of addExternalFirewall etc. is too generic, so I thought
> if you need them as well, you should able to merge your version and SRX
> version into one command.
>
> --Sheng
>
>
> On Fri, Nov 15, 2013 at 9:47 AM, Will Stevens <ws...@cloudops.com>wrote:
>
>> I have looked into this briefly.  It appears that these commands are not
>> needed, so I will remove them and only keep the Palo Alto specific commands.
>>
>> addPaloAltoFirewall=1
>>  deletePaloAltoFirewall=1
>>  configurePaloAltoFirewall=1
>>  listPaloAltoFirewalls=1
>>  listPaloAltoFirewallNetworks=1
>>
>> Cheers,
>>
>> Will
>>
>>
>>
>>
>> On Thu, Nov 14, 2013 at 11:49 PM, Will Stevens <ws...@cloudops.com>wrote:
>>
>>> Yes, I will look into the naming of the API commands in the Palo Alto
>>> plugin.  When you say 'they are supposed to be merged as one command', what
>>> do you mean?
>>>
>>> These were the commands that I exposed.
>>>
>>>   #### Palo Alto firewall commands
>>>  addExternalFirewall=1
>>>  deleteExternalFirewall=1
>>>  listExternalFirewalls=1
>>>
>>> addPaloAltoFirewall=1
>>>  deletePaloAltoFirewall=1
>>>  configurePaloAltoFirewall=1
>>>  listPaloAltoFirewalls=1
>>>  listPaloAltoFirewallNetworks=1
>>>
>>> When I started working on this I was using the SRX as a model as it was
>>> the only documentation I had at the time.  I was under the impression that
>>> I was supposed to override those commands.  Once I got things working I did
>>> not go back over this, so I am sure I can improve this aspect of the plugin.
>>>
>>> Thanks,
>>>
>>> Will
>>>
>>>
>>>
>>>
>>> On Thu, Nov 14, 2013 at 9:01 PM, Sheng Yang <sh...@yasker.org> wrote:
>>>
>>>> Hi Will,
>>>>
>>>> Could you check on Palo Alto's duplicate api commands? They suppose to
>>>> be merged as one command I think.
>>>>
>>>> BTW, how can this works? Did it broke SRX?
>>>>
>>>> --Sheng
>>>>
>>>>
>>>> On Thu, Nov 14, 2013 at 2:55 AM, Santhosh Edukulla <
>>>> santhosh.edukulla@citrix.com> wrote:
>>>>
>>>>> Team,
>>>>>
>>>>> While running code coverage analysis with sonar for integration tests,
>>>>> based upon the errors thrown, i could see the below issues\notes with CS
>>>>> project.
>>>>>
>>>>> Issue1:
>>>>>
>>>>> The coverage tool is complaining about duplicate sources  for below
>>>>> files. These are available with same name under folders
>>>>> ./cloudstack/plugins/network-elements/juniper-srx and as well under
>>>>> ..../palo-alto.
>>>>>
>>>>>
>>>>> ./cloudstack/plugins/network-elements/palo-alto/src/com/cloud/api/commands/AddExternalFirewallCmd.java
>>>>>
>>>>> ./cloudstack/plugins/network-elements/palo-alto/src/com/cloud/api/commands/DeleteExternalFirewallCmd.java
>>>>>
>>>>> ./cloudstack/plugins/network-elements/palo-alto/src/com/cloud/api/commands/ListExternalFirewallCmd.java
>>>>>
>>>>> I renamed one while running analysis to a different name.  It
>>>>> proceeded further with its analysis once renamed.
>>>>>
>>>>> Is it intentional to have same name or can be renamed?
>>>>>
>>>>> Issue2:
>>>>>
>>>>> The source directory does not correspond to package declaration for
>>>>>  code files under
>>>>>
>>>>> /root/softwares/cscode/cloudstack/services/console-proxy-rdp/rdpconsole/src/main/java/rdpclient/
>>>>>
>>>>> This error when compared to other files in the similar path has a
>>>>> different package convention and usage.
>>>>>
>>>>> /root/softwares/cscode/cloudstack/services/secondary-storage/src/org/apache/cloudstack/storage/
>>>>>
>>>>> Changing tool configuration properties worked to over come this, but
>>>>> Is it intentional to have a different package structure for rdpclient code
>>>>> base against others?
>>>>>
>>>>>
>>>>>
>>>>> Thanks!
>>>>> Santhosh
>>>>>
>>>>
>>>>
>>>
>>
>

Re: Coverage Analysis: Few Issues

Posted by Sheng Yang <sh...@yasker.org>.
Sure, if you don't need these addExternalFirewall etc., you can just remove
them.

Because the name of addExternalFirewall etc. is too generic, so I thought
if you need them as well, you should able to merge your version and SRX
version into one command.

--Sheng


On Fri, Nov 15, 2013 at 9:47 AM, Will Stevens <ws...@cloudops.com> wrote:

> I have looked into this briefly.  It appears that these commands are not
> needed, so I will remove them and only keep the Palo Alto specific commands.
>
> addPaloAltoFirewall=1
>  deletePaloAltoFirewall=1
>  configurePaloAltoFirewall=1
>  listPaloAltoFirewalls=1
>  listPaloAltoFirewallNetworks=1
>
> Cheers,
>
> Will
>
>
>
>
> On Thu, Nov 14, 2013 at 11:49 PM, Will Stevens <ws...@cloudops.com>wrote:
>
>> Yes, I will look into the naming of the API commands in the Palo Alto
>> plugin.  When you say 'they are supposed to be merged as one command', what
>> do you mean?
>>
>> These were the commands that I exposed.
>>
>>   #### Palo Alto firewall commands
>>  addExternalFirewall=1
>>  deleteExternalFirewall=1
>>  listExternalFirewalls=1
>>
>> addPaloAltoFirewall=1
>>  deletePaloAltoFirewall=1
>>  configurePaloAltoFirewall=1
>>  listPaloAltoFirewalls=1
>>  listPaloAltoFirewallNetworks=1
>>
>> When I started working on this I was using the SRX as a model as it was
>> the only documentation I had at the time.  I was under the impression that
>> I was supposed to override those commands.  Once I got things working I did
>> not go back over this, so I am sure I can improve this aspect of the plugin.
>>
>> Thanks,
>>
>> Will
>>
>>
>>
>>
>> On Thu, Nov 14, 2013 at 9:01 PM, Sheng Yang <sh...@yasker.org> wrote:
>>
>>> Hi Will,
>>>
>>> Could you check on Palo Alto's duplicate api commands? They suppose to
>>> be merged as one command I think.
>>>
>>> BTW, how can this works? Did it broke SRX?
>>>
>>> --Sheng
>>>
>>>
>>> On Thu, Nov 14, 2013 at 2:55 AM, Santhosh Edukulla <
>>> santhosh.edukulla@citrix.com> wrote:
>>>
>>>> Team,
>>>>
>>>> While running code coverage analysis with sonar for integration tests,
>>>> based upon the errors thrown, i could see the below issues\notes with CS
>>>> project.
>>>>
>>>> Issue1:
>>>>
>>>> The coverage tool is complaining about duplicate sources  for below
>>>> files. These are available with same name under folders
>>>> ./cloudstack/plugins/network-elements/juniper-srx and as well under
>>>> ..../palo-alto.
>>>>
>>>>
>>>> ./cloudstack/plugins/network-elements/palo-alto/src/com/cloud/api/commands/AddExternalFirewallCmd.java
>>>>
>>>> ./cloudstack/plugins/network-elements/palo-alto/src/com/cloud/api/commands/DeleteExternalFirewallCmd.java
>>>>
>>>> ./cloudstack/plugins/network-elements/palo-alto/src/com/cloud/api/commands/ListExternalFirewallCmd.java
>>>>
>>>> I renamed one while running analysis to a different name.  It proceeded
>>>> further with its analysis once renamed.
>>>>
>>>> Is it intentional to have same name or can be renamed?
>>>>
>>>> Issue2:
>>>>
>>>> The source directory does not correspond to package declaration for
>>>>  code files under
>>>>
>>>> /root/softwares/cscode/cloudstack/services/console-proxy-rdp/rdpconsole/src/main/java/rdpclient/
>>>>
>>>> This error when compared to other files in the similar path has a
>>>> different package convention and usage.
>>>>
>>>> /root/softwares/cscode/cloudstack/services/secondary-storage/src/org/apache/cloudstack/storage/
>>>>
>>>> Changing tool configuration properties worked to over come this, but Is
>>>> it intentional to have a different package structure for rdpclient code
>>>> base against others?
>>>>
>>>>
>>>>
>>>> Thanks!
>>>> Santhosh
>>>>
>>>
>>>
>>
>

Re: Coverage Analysis: Few Issues

Posted by Will Stevens <ws...@cloudops.com>.
I have looked into this briefly.  It appears that these commands are not
needed, so I will remove them and only keep the Palo Alto specific commands.

addPaloAltoFirewall=1
deletePaloAltoFirewall=1
configurePaloAltoFirewall=1
listPaloAltoFirewalls=1
listPaloAltoFirewallNetworks=1

Cheers,

Will




On Thu, Nov 14, 2013 at 11:49 PM, Will Stevens <ws...@cloudops.com>wrote:

> Yes, I will look into the naming of the API commands in the Palo Alto
> plugin.  When you say 'they are supposed to be merged as one command', what
> do you mean?
>
> These were the commands that I exposed.
>
>   #### Palo Alto firewall commands
>  addExternalFirewall=1
>  deleteExternalFirewall=1
>  listExternalFirewalls=1
>
> addPaloAltoFirewall=1
>  deletePaloAltoFirewall=1
>  configurePaloAltoFirewall=1
>  listPaloAltoFirewalls=1
>  listPaloAltoFirewallNetworks=1
>
> When I started working on this I was using the SRX as a model as it was
> the only documentation I had at the time.  I was under the impression that
> I was supposed to override those commands.  Once I got things working I did
> not go back over this, so I am sure I can improve this aspect of the plugin.
>
> Thanks,
>
> Will
>
>
>
>
> On Thu, Nov 14, 2013 at 9:01 PM, Sheng Yang <sh...@yasker.org> wrote:
>
>> Hi Will,
>>
>> Could you check on Palo Alto's duplicate api commands? They suppose to be
>> merged as one command I think.
>>
>> BTW, how can this works? Did it broke SRX?
>>
>> --Sheng
>>
>>
>> On Thu, Nov 14, 2013 at 2:55 AM, Santhosh Edukulla <
>> santhosh.edukulla@citrix.com> wrote:
>>
>>> Team,
>>>
>>> While running code coverage analysis with sonar for integration tests,
>>> based upon the errors thrown, i could see the below issues\notes with CS
>>> project.
>>>
>>> Issue1:
>>>
>>> The coverage tool is complaining about duplicate sources  for below
>>> files. These are available with same name under folders
>>> ./cloudstack/plugins/network-elements/juniper-srx and as well under
>>> ..../palo-alto.
>>>
>>>
>>> ./cloudstack/plugins/network-elements/palo-alto/src/com/cloud/api/commands/AddExternalFirewallCmd.java
>>>
>>> ./cloudstack/plugins/network-elements/palo-alto/src/com/cloud/api/commands/DeleteExternalFirewallCmd.java
>>>
>>> ./cloudstack/plugins/network-elements/palo-alto/src/com/cloud/api/commands/ListExternalFirewallCmd.java
>>>
>>> I renamed one while running analysis to a different name.  It proceeded
>>> further with its analysis once renamed.
>>>
>>> Is it intentional to have same name or can be renamed?
>>>
>>> Issue2:
>>>
>>> The source directory does not correspond to package declaration for
>>>  code files under
>>>
>>> /root/softwares/cscode/cloudstack/services/console-proxy-rdp/rdpconsole/src/main/java/rdpclient/
>>>
>>> This error when compared to other files in the similar path has a
>>> different package convention and usage.
>>>
>>> /root/softwares/cscode/cloudstack/services/secondary-storage/src/org/apache/cloudstack/storage/
>>>
>>> Changing tool configuration properties worked to over come this, but Is
>>> it intentional to have a different package structure for rdpclient code
>>> base against others?
>>>
>>>
>>>
>>> Thanks!
>>> Santhosh
>>>
>>
>>
>

Re: Coverage Analysis: Few Issues

Posted by Will Stevens <ws...@cloudops.com>.
Yes, I will look into the naming of the API commands in the Palo Alto
plugin.  When you say 'they are supposed to be merged as one command', what
do you mean?

These were the commands that I exposed.

#### Palo Alto firewall commands
addExternalFirewall=1
deleteExternalFirewall=1
listExternalFirewalls=1

addPaloAltoFirewall=1
deletePaloAltoFirewall=1
configurePaloAltoFirewall=1
listPaloAltoFirewalls=1
listPaloAltoFirewallNetworks=1

When I started working on this I was using the SRX as a model as it was the
only documentation I had at the time.  I was under the impression that I
was supposed to override those commands.  Once I got things working I did
not go back over this, so I am sure I can improve this aspect of the plugin.

Thanks,

Will




On Thu, Nov 14, 2013 at 9:01 PM, Sheng Yang <sh...@yasker.org> wrote:

> Hi Will,
>
> Could you check on Palo Alto's duplicate api commands? They suppose to be
> merged as one command I think.
>
> BTW, how can this works? Did it broke SRX?
>
> --Sheng
>
>
> On Thu, Nov 14, 2013 at 2:55 AM, Santhosh Edukulla <
> santhosh.edukulla@citrix.com> wrote:
>
>> Team,
>>
>> While running code coverage analysis with sonar for integration tests,
>> based upon the errors thrown, i could see the below issues\notes with CS
>> project.
>>
>> Issue1:
>>
>> The coverage tool is complaining about duplicate sources  for below
>> files. These are available with same name under folders
>> ./cloudstack/plugins/network-elements/juniper-srx and as well under
>> ..../palo-alto.
>>
>>
>> ./cloudstack/plugins/network-elements/palo-alto/src/com/cloud/api/commands/AddExternalFirewallCmd.java
>>
>> ./cloudstack/plugins/network-elements/palo-alto/src/com/cloud/api/commands/DeleteExternalFirewallCmd.java
>>
>> ./cloudstack/plugins/network-elements/palo-alto/src/com/cloud/api/commands/ListExternalFirewallCmd.java
>>
>> I renamed one while running analysis to a different name.  It proceeded
>> further with its analysis once renamed.
>>
>> Is it intentional to have same name or can be renamed?
>>
>> Issue2:
>>
>> The source directory does not correspond to package declaration for  code
>> files under
>>
>> /root/softwares/cscode/cloudstack/services/console-proxy-rdp/rdpconsole/src/main/java/rdpclient/
>>
>> This error when compared to other files in the similar path has a
>> different package convention and usage.
>>
>> /root/softwares/cscode/cloudstack/services/secondary-storage/src/org/apache/cloudstack/storage/
>>
>> Changing tool configuration properties worked to over come this, but Is
>> it intentional to have a different package structure for rdpclient code
>> base against others?
>>
>>
>>
>> Thanks!
>> Santhosh
>>
>
>