[jira] [Commented] (CLOUDSTACK-8590) Refactoring NiciraNVP resource
[ https://issues.apache.org/jira/browse/CLOUDSTACK-8590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14608225#comment-14608225 ] ASF GitHub Bot commented on CLOUDSTACK-8590: GitHub user wilderrodrigues opened a pull request: https://github.com/apache/cloudstack/pull/546 CLOUDSTACK-8590 Refactoring NiciraNVP resource Hi there, The motivation behind the refactor of the NiciraNvpResource class was to decouple the execute of the commands, which was handled into several private methods in the class. With the refactor, we were able to decouple all the commands by wrapping them with another class. The extra classes are declared with the ResourceWrapper annotation, which makes possible to get all the classes loaded into a map inside the implementation of the RequestWrapper class, in that case the NiciraNvpRequestWrapper. New tests were added in order to increase test coverage. The current status is: * Resource package: 86.7% coverage * Utils package: 95.1% coverage * Wrapper package: 97.5% coverage In addition, the previous retry mechanism has been improved. Instead of having an instance variable to control the retries and the extra methods with the number of retries passed as parameter, we now have a Map which holds the commands and the number of retry. Once the number of retries reaches ZERO, the given command is removed from the Map. An issue has been created in order to follow up with this refactor: https://issues.apache.org/jira/browse/CLOUDSTACK-8590 The changes do not affect the behaviour of the NiciraNvpResource. As a proof of that, all the existing tests worked without any change. This is a pure structural change on the code. This PR consists of 11 commits. I tried to combine the work on the command wrappers, retry mechanism, unit tests and general stuff. Cheers, Wilder You can merge this pull request into a Git repository by running: $ git pull https://github.com/schubergphilis/cloudstack refactor/nicira_resource Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cloudstack/pull/546.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #546 commit fd27b7f96b7e257e4220d072f91dafb2bbfe4fa4 Author: wilderrodrigues Date: 2015-06-22T13:57:15Z CLOUDSTACK-8590 - Refactoring NiciraNVP resource - Adding the NiciraNvpWrapper - This class will keep track of all Wrappers of the Nicira NVP Plugin commit 2e0d01e6e90d100a8fc0eeebac22ef4d4f644d2c Author: wilderrodrigues Date: 2015-06-23T07:20:47Z CLOUDSTACK-8590 - Refactoring NiciraNVP resource - Refactoring NiciraNvpResource - Added NiciraNvpRequestWrapper - Removing 1 execute methods form NiciraNvpResource - Added 1 unit test commit 8b9b3dc8a28dba60ac42cdaef51585c097d1223f Author: wilderrodrigues Date: 2015-06-23T07:38:56Z CLOUDSTACK-8590 - Refactoring NiciraNVP resource - Refactoring NiciraNvpResource - Added NiciraNvpMaintainCommandWrapper - Removing 1 execute methods form NiciraNvpResource - Added 1 unit test commit bc349571e5c896f24b7fab941aa5a0832ebe0ce3 Author: wilderrodrigues Date: 2015-06-23T08:47:23Z CLOUDSTACK-8590 - Refactoring NiciraNVP resource - Refactoring NiciraNvpResource - Added NiciraNvpUtilities and NiciraNvpCreateLogicalSwitchCommandWrapper - Removing 1 execute methods form NiciraNvpResource - Added 1 unit test commit ea7c38311cb3ef1005d5e3a1d2c6c8ebe373b10e Author: wilderrodrigues Date: 2015-06-30T07:47:57Z CLOUDSTACK-8590 - Refactoring NiciraNVP resource - Adding command wrappers NiciraNvpCreateLogicalSwitchCommandWrapper and NiciraNvpDeleteLogicalSwitchCommandWrapper - Refactoring the retry mechanism - Applying the new retry mechanism to current wrappers and old methods in NiciraNvpResource - Adding 2 tests - Fixing the testRetries() in NiciraNvpResourceTest class commit 40a320a267d4bdfabc03a76e59f12087fbd5c776 Author: wilderrodrigues Date: 2015-06-30T08:22:25Z CLOUDSTACK-8590 - Refactoring NiciraNVP resource - Adding NiciraNvpCreateLogicalSwitchPortCommandWrapper - Removing unsued field from NiciraNvpResourceTest commit 2a8332870d2528ffbef6a6fd3ab6f0ba3e0880d7 Author: wilderrodrigues Date: 2015-06-30T09:07:59Z CLOUDSTACK-8590 - Refactoring NiciraNVP resource - Adding NiciraNvpDeleteLogicalSwitchPortCommandWrapper commit 6f4f8a07991ab4e320842bb8c05c6d17cb7d2c06 Author: wilderrodrigues Date: 2015-06-30T09:18:58Z CLOUDSTACK-8590 - Refactoring NiciraNVP resource - Adding NiciraNvpCreateLogicalRouterComma
[jira] [Commented] (CLOUDSTACK-8590) Refactoring NiciraNVP resource
[ https://issues.apache.org/jira/browse/CLOUDSTACK-8590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14610074#comment-14610074 ] ASF GitHub Bot commented on CLOUDSTACK-8590: Github user bhaisaab commented on the pull request: https://github.com/apache/cloudstack/pull/546#issuecomment-117653714 @wilderrodrigues LGTM, and Travis is green though there is no way for me to test it against the real h/w. Please go ahead with the merge. > Refactoring NiciraNVP resource > -- > > Key: CLOUDSTACK-8590 > URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8590 > Project: CloudStack > Issue Type: Improvement > Security Level: Public(Anyone can view this level - this is the > default.) > Components: Network Controller >Reporter: Wilder Rodrigues >Assignee: Wilder Rodrigues > Fix For: 4.6.0 > > > Apply the same technique that was used for the refactoring of the Libvirt and > XenServer hypervisors. > All the commands will be wrapped by a ResourceWrapper annotated class. This > will reduce the amount of private methods in the NiciraNvpResource class and > also increase its testability, maintainability, extensibility and > readability -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CLOUDSTACK-8590) Refactoring NiciraNVP resource
[ https://issues.apache.org/jira/browse/CLOUDSTACK-8590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14610159#comment-14610159 ] ASF subversion and git services commented on CLOUDSTACK-8590: - Commit b4ce81ab6ce2a007cc787f3658934cdd9745c359 in cloudstack's branch refs/heads/master from [~wilder.rodrigues] [ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=b4ce81a ] CLOUDSTACK-8590 - Refactoring NiciraNVP resource - Adding NiciraNvpDeleteLogicalSwitchPortCommandWrapper Signed-off-by: wilderrodrigues > Refactoring NiciraNVP resource > -- > > Key: CLOUDSTACK-8590 > URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8590 > Project: CloudStack > Issue Type: Improvement > Security Level: Public(Anyone can view this level - this is the > default.) > Components: Network Controller >Reporter: Wilder Rodrigues >Assignee: Wilder Rodrigues > Fix For: 4.6.0 > > > Apply the same technique that was used for the refactoring of the Libvirt and > XenServer hypervisors. > All the commands will be wrapped by a ResourceWrapper annotated class. This > will reduce the amount of private methods in the NiciraNvpResource class and > also increase its testability, maintainability, extensibility and > readability -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CLOUDSTACK-8590) Refactoring NiciraNVP resource
[ https://issues.apache.org/jira/browse/CLOUDSTACK-8590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14610189#comment-14610189 ] ASF subversion and git services commented on CLOUDSTACK-8590: - Commit 96ad6f6ccd0c18561172ef2e1cbcde28291a6df7 in cloudstack's branch refs/heads/master from [~wilder.rodrigues] [ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=96ad6f6 ] CLOUDSTACK-8590 - Refactoring NiciraNVP resource - Change the retry() method to return Answer.createUnsupportedCommandAnswer(command) instead of throwing an exception Signed-off-by: wilderrodrigues This closes #546 > Refactoring NiciraNVP resource > -- > > Key: CLOUDSTACK-8590 > URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8590 > Project: CloudStack > Issue Type: Improvement > Security Level: Public(Anyone can view this level - this is the > default.) > Components: Network Controller >Reporter: Wilder Rodrigues >Assignee: Wilder Rodrigues > Fix For: 4.6.0 > > > Apply the same technique that was used for the refactoring of the Libvirt and > XenServer hypervisors. > All the commands will be wrapped by a ResourceWrapper annotated class. This > will reduce the amount of private methods in the NiciraNvpResource class and > also increase its testability, maintainability, extensibility and > readability -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CLOUDSTACK-8590) Refactoring NiciraNVP resource
[ https://issues.apache.org/jira/browse/CLOUDSTACK-8590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14610177#comment-14610177 ] ASF subversion and git services commented on CLOUDSTACK-8590: - Commit 644458f5aca17e44381fd7393cefe7da42a56803 in cloudstack's branch refs/heads/master from [~wilder.rodrigues] [ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=644458f ] CLOUDSTACK-8590 - Refactoring NiciraNVP resource - Adding remaining command wrappers Signed-off-by: wilderrodrigues > Refactoring NiciraNVP resource > -- > > Key: CLOUDSTACK-8590 > URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8590 > Project: CloudStack > Issue Type: Improvement > Security Level: Public(Anyone can view this level - this is the > default.) > Components: Network Controller >Reporter: Wilder Rodrigues >Assignee: Wilder Rodrigues > Fix For: 4.6.0 > > > Apply the same technique that was used for the refactoring of the Libvirt and > XenServer hypervisors. > All the commands will be wrapped by a ResourceWrapper annotated class. This > will reduce the amount of private methods in the NiciraNvpResource class and > also increase its testability, maintainability, extensibility and > readability -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CLOUDSTACK-8590) Refactoring NiciraNVP resource
[ https://issues.apache.org/jira/browse/CLOUDSTACK-8590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14610190#comment-14610190 ] ASF GitHub Bot commented on CLOUDSTACK-8590: Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/546 > Refactoring NiciraNVP resource > -- > > Key: CLOUDSTACK-8590 > URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8590 > Project: CloudStack > Issue Type: Improvement > Security Level: Public(Anyone can view this level - this is the > default.) > Components: Network Controller >Reporter: Wilder Rodrigues >Assignee: Wilder Rodrigues > Fix For: 4.6.0 > > > Apply the same technique that was used for the refactoring of the Libvirt and > XenServer hypervisors. > All the commands will be wrapped by a ResourceWrapper annotated class. This > will reduce the amount of private methods in the NiciraNvpResource class and > also increase its testability, maintainability, extensibility and > readability -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CLOUDSTACK-8590) Refactoring NiciraNVP resource
[ https://issues.apache.org/jira/browse/CLOUDSTACK-8590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14610185#comment-14610185 ] ASF subversion and git services commented on CLOUDSTACK-8590: - Commit c6013303e9837c12922fbed28836d34991ec6ab3 in cloudstack's branch refs/heads/master from [~wilder.rodrigues] [ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=c601330 ] CLOUDSTACK-8590 - Refactoring NiciraNVP resource - Increasing test coverage - resource package: 86.7% - utils package: 95.1% - wrapper package: 97.5% Signed-off-by: wilderrodrigues > Refactoring NiciraNVP resource > -- > > Key: CLOUDSTACK-8590 > URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8590 > Project: CloudStack > Issue Type: Improvement > Security Level: Public(Anyone can view this level - this is the > default.) > Components: Network Controller >Reporter: Wilder Rodrigues >Assignee: Wilder Rodrigues > Fix For: 4.6.0 > > > Apply the same technique that was used for the refactoring of the Libvirt and > XenServer hypervisors. > All the commands will be wrapped by a ResourceWrapper annotated class. This > will reduce the amount of private methods in the NiciraNvpResource class and > also increase its testability, maintainability, extensibility and > readability -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CLOUDSTACK-8590) Refactoring NiciraNVP resource
[ https://issues.apache.org/jira/browse/CLOUDSTACK-8590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14610168#comment-14610168 ] ASF subversion and git services commented on CLOUDSTACK-8590: - Commit b54126918b4c4c620e08b0215dce9bd8da8715fd in cloudstack's branch refs/heads/master from [~wilder.rodrigues] [ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=b541269 ] CLOUDSTACK-8590 - Refactoring NiciraNVP resource - Adding NiciraNvpCreateLogicalRouterCommandWrapper - Adding NiciraNvpDeleteLogicalSwitchPortCommandWrapper - Adding NiciraNvpFindLogicalSwitchPortCommandWrapper - Adding NiciraNvpUpdateLogicalSwitchPortCommandWrapper - Decoupling private methods from NiciraNvpResource Signed-off-by: wilderrodrigues > Refactoring NiciraNVP resource > -- > > Key: CLOUDSTACK-8590 > URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8590 > Project: CloudStack > Issue Type: Improvement > Security Level: Public(Anyone can view this level - this is the > default.) > Components: Network Controller >Reporter: Wilder Rodrigues >Assignee: Wilder Rodrigues > Fix For: 4.6.0 > > > Apply the same technique that was used for the refactoring of the Libvirt and > XenServer hypervisors. > All the commands will be wrapped by a ResourceWrapper annotated class. This > will reduce the amount of private methods in the NiciraNvpResource class and > also increase its testability, maintainability, extensibility and > readability -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CLOUDSTACK-8590) Refactoring NiciraNVP resource
[ https://issues.apache.org/jira/browse/CLOUDSTACK-8590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14611419#comment-14611419 ] ASF GitHub Bot commented on CLOUDSTACK-8590: Github user karuturi commented on the pull request: https://github.com/apache/cloudstack/pull/546#issuecomment-117896507 shouldnt we wait for another review? > Refactoring NiciraNVP resource > -- > > Key: CLOUDSTACK-8590 > URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8590 > Project: CloudStack > Issue Type: Improvement > Security Level: Public(Anyone can view this level - this is the > default.) > Components: Network Controller >Reporter: Wilder Rodrigues >Assignee: Wilder Rodrigues > Fix For: 4.6.0 > > > Apply the same technique that was used for the refactoring of the Libvirt and > XenServer hypervisors. > All the commands will be wrapped by a ResourceWrapper annotated class. This > will reduce the amount of private methods in the NiciraNvpResource class and > also increase its testability, maintainability, extensibility and > readability -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CLOUDSTACK-8590) Refactoring NiciraNVP resource
[ https://issues.apache.org/jira/browse/CLOUDSTACK-8590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14611844#comment-14611844 ] ASF GitHub Bot commented on CLOUDSTACK-8590: Github user remibergsma commented on the pull request: https://github.com/apache/cloudstack/pull/546#issuecomment-118009819 @karuturi I agree, we should only proceed after two LGTMs. Let met give the second LGTM, after the fact. > Refactoring NiciraNVP resource > -- > > Key: CLOUDSTACK-8590 > URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8590 > Project: CloudStack > Issue Type: Improvement > Security Level: Public(Anyone can view this level - this is the > default.) > Components: Network Controller >Reporter: Wilder Rodrigues >Assignee: Wilder Rodrigues > Fix For: 4.6.0 > > > Apply the same technique that was used for the refactoring of the Libvirt and > XenServer hypervisors. > All the commands will be wrapped by a ResourceWrapper annotated class. This > will reduce the amount of private methods in the NiciraNvpResource class and > also increase its testability, maintainability, extensibility and > readability -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CLOUDSTACK-8590) Refactoring NiciraNVP resource
[ https://issues.apache.org/jira/browse/CLOUDSTACK-8590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14611864#comment-14611864 ] ASF GitHub Bot commented on CLOUDSTACK-8590: Github user wilderrodrigues commented on the pull request: https://github.com/apache/cloudstack/pull/546#issuecomment-118011859 @remibergsma and @karuturi You are right. I believe both @bhaisaab and I forgot that. Once he said "go ahead with the merge" I just merged the thing. Apologies. Cheers, Wilder > Refactoring NiciraNVP resource > -- > > Key: CLOUDSTACK-8590 > URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8590 > Project: CloudStack > Issue Type: Improvement > Security Level: Public(Anyone can view this level - this is the > default.) > Components: Network Controller >Reporter: Wilder Rodrigues >Assignee: Wilder Rodrigues > Fix For: 4.6.0 > > > Apply the same technique that was used for the refactoring of the Libvirt and > XenServer hypervisors. > All the commands will be wrapped by a ResourceWrapper annotated class. This > will reduce the amount of private methods in the NiciraNvpResource class and > also increase its testability, maintainability, extensibility and > readability -- This message was sent by Atlassian JIRA (v6.3.4#6332)
