[GitHub] cloudstack pull request: CLOUDSTACK-8656: tests ignoring exception...
Github user borisroman commented on the pull request: https://github.com/apache/cloudstack/pull/850#issuecomment-141662154 @DaanHoogland Why log when an exception is thrown? In a test when an exception is thrown it should either fail, or when @Test(expected = --exception.class--) is defined succeed. Right? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-8805: Domains become inactive ...
Github user rafaelweingartner commented on the pull request:
https://github.com/apache/cloudstack/pull/775#issuecomment-141683980
To get this PR going, you could extract the code â"%" + "replace(" +
domainHandle.getPath() + ", '%', '[%]')" + "%"â of line 357 to a method and
then create a test case to that method. The test case would check if the
â%â as part of a domainPath is being properly handled either replacing it
with \% (escaping with) or as you did using the replace function of the
database.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8805: Domains become inactive ...
Github user nitin-maharana commented on the pull request: https://github.com/apache/cloudstack/pull/775#issuecomment-141687462 Thanks for the suggestion. yes, I will follow the same. I will make an another PR with the change and test file. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-8864: Not able to add TCP port...
GitHub user SudharmaJain opened a pull request: https://github.com/apache/cloudstack/pull/851 CLOUDSTACK-8864: Not able to add TCP port forwarding rule in VPN for ⦠â¦specific ports Setting port forwarding rules for port 500,1701 and 4500 after enabling VPN, gives the error message "The range specified, , conflicts with rule which has ." This happens because the rules added for vpn doesn't have a matching condition to allow port forwarding rules. Added a unit test to verify the detectRulesConflict function of FirewallManagerImpl. You can merge this pull request into a Git repository by running: $ git pull https://github.com/SudharmaJain/cloudstack cs-8864 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cloudstack/pull/851.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 #851 commit 96c38bf491d81e41975dddbfc3c87716293c7bdf Author: SudharmaJain Date: 2015-09-19T18:10:21Z CLOUDSTACK-8864: Not able to add TCP port forwarding rule in VPN for specific ports --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Removal of DefaultUserAuthenticator empty...
GitHub user alexandrelimassantana opened a pull request: https://github.com/apache/cloudstack/pull/852 Removal of DefaultUserAuthenticator empty class. The DefaultUserAuthenticator is an empty class, extending from the AdapterBase and implementing the UserAuthenticator interface. The class is not being used as a marker and it's sole use is to be extended by other UserAuthenticators. Noticing that the class had no purpose, I removed it and made it's children extend from it's superclass and implement it's interface instead. You can merge this pull request into a Git repository by running: $ git pull https://github.com/rafaelweingartner/cloudstack master-lrg-cs-hackday-005 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cloudstack/pull/852.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 #852 commit 91daba381d95ac4556a60db62799fdeee0da4445 Author: Alexandre de Limas Santana Date: 2015-09-19T18:30:27Z Removal of DefaultUserAuthenticator empty class. The DefaultUserAuthenticator is an empty class, extending from the AdapterBase and implementing the UserAuthenticator interface. The class is not being used as a marker and it's sole use is to be extended by other UserAuthenticators. Noticing that the class had no purpose, I removed it and made it's children extend from it's superclass and implement it's interface instead. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Removed unnecessary @Local annotations an...
GitHub user cristofolini opened a pull request: https://github.com/apache/cloudstack/pull/853 Removed unnecessary @Local annotations and their respective imports Following @rafaelweingartner 's findings in PR #714 that many spring beans contained an @Local annotation, we've decided to remove said annotations and their imports seeking a reduction of a few hundred lines of useless code. You can merge this pull request into a Git repository by running: $ git pull https://github.com/rafaelweingartner/cloudstack master-lrg-cs-hackday-006 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cloudstack/pull/853.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 #853 commit c0ff4ed2c7f6e9282068fbabb0fad41ececa6002 Author: cristofolini Date: 2015-09-19T19:26:46Z Removed unnecessary @Local annotations and their respective imports from the ComponentLifecycleBase class and its subclasses. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Fixed return type Void to void in DataMot...
GitHub user rodrigo93 opened a pull request: https://github.com/apache/cloudstack/pull/854 Fixed return type Void to void in DataMotionStrategy. The main changes are: - Changing methods âVoidâ to âvoidâ. - Removal of the method âVoid copyAsync(DataObject srcData, DataObject destData, AsyncCompletionCallback callback)â that was never used. We noticed that methods form that class are using the return type Void with capital V. This way that method has to return a null value at the end. The alterations were made at the following classes and their respective methods: 1. /cloud-plugin-hypervisor-hyperv/src/org/apache/cloudstack/storage/motion/HypervStorageMotionStrategy.java 2. /cloud-engine-api/src/org/apache/cloudstack/engine/subsystem/api/storage/DataMotionStrategy.java 3. /cloud-engine-storage-datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java 4. /cloud-engine-storage-datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java 5. /cloud-plugin-hypervisor-simulator/src/org/apache/cloudstack/storage/motion/SimulatorDataMotionStrategy.java 6. /cloud-plugin-hypervisor-xenserver/src/org/apache/cloudstack/storage/motion/XenServerStorageMotionStrategy.java We also removed the following method from the class âorg.apache.cloudstack.engine.subsystem.api.storage.DataMotionStrategyâ because it was never called nor used. Method: âVoid copyAsync(DataObject srcData, DataObject destData, AsyncCompletionCallback callback)â You can merge this pull request into a Git repository by running: $ git pull https://github.com/rafaelweingartner/cloudstack master-lrg-cs-hackday-002 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cloudstack/pull/854.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 #854 commit 82bf6c0058ed5d9bf7a732a66f7cbcfd3a3cc22a Author: Rodrigo Marques Date: 2015-09-19T19:57:20Z Fixed return type Void to void in DataMotionStrategy. The main changes are: - Changing methods âVoidâ to âvoidâ. - Removal of the method âVoid copyAsync(DataObject srcData, DataObject destData, AsyncCompletionCallback callback)â that was never used. We noticed that methods form that class are using the return type Void with capital V. This way that method has to return a null value at the end. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Removal of class AgentBasedStandaloneCons...
GitHub user alexandrelimassantana opened a pull request: https://github.com/apache/cloudstack/pull/855 Removal of class AgentBasedStandaloneConsoleProxyManager The AgentBasedStandaloneConsoleProxyManager class is neither manually instatiated anywhere in the code nor via Spring. Further checking in the interface which the class implements, shows that the use for classes implementing the EventBus interface relies on instances generated by the Spring framework, which discards the possibility of the class being created by EJB. Being the class useless, it was discarded. You can merge this pull request into a Git repository by running: $ git pull https://github.com/rafaelweingartner/cloudstack master-lrg-cs-hackday-007 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cloudstack/pull/855.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 #855 commit 5470cc4e76c9753b3e2894ff50ff6940443f2bd6 Author: Alexandre de Limas Santana Date: 2015-09-19T19:57:04Z Removal of class AgentBasedStandaloneConsoleProxyManager The AgentBasedStandaloneConsoleProxyManager class is neither manually instatiated anywhere in the code nor via Spring. Further checking in the interface which the class implements, shows that the use for classes implementing the EventBus interface relies on instances generated by the Spring framework, which discards the possibility of the class being created by EJB. Being the class useless, it was discarded. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Removed the empty class 'com.cloud.deploy...
GitHub user pedro-martins opened a pull request: https://github.com/apache/cloudstack/pull/856 Removed the empty class 'com.cloud.deploy.PlannerBase' Removed the PlannerBase class because it is does not bring contribution to ACS' code. We changed com.cloud.deploy.FirstFitPlanner, now it doesnât extends the PlannerBase and implements the com.cloud.deploy.DeploymentPlanner. We also removed the method com.cloud.deploy.DeploymentPlanner.check(VirtualMachineProfile, DeploymentPlan, DeployDestination, ExcludeList) that was not used anywhere. Additionally, we removed the â_â from FirstFitPlanner's attributes name, in order to have them in a more standard way. You can merge this pull request into a Git repository by running: $ git pull https://github.com/rafaelweingartner/cloudstack master-lrg-cs-hackday-004 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cloudstack/pull/856.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 #856 commit d5a22526f1fe895d641d13ff0dea8505e2d949a2 Author: pedro-martins Date: 2015-09-19T20:10:21Z Removed class com.cloud.deploy.PlannerBase removed method com.cloud.deploy.DeploymentPlanner.check(VirtualMachineProfile, DeploymentPlan, DeployDestination, ExcludeList) modified the com.cloud.deploy.FirstFitPlanner for extends AdapterBase and implements DeploymentPlanner removed "_" from variables name --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Fixed return type Void to void in DataMot...
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/854#issuecomment-141711633 Hi @rodrigo93, It seems there is a problem with your PR. Please in Eclipse go: Window>Preferences>Java> Editor>Save Actions: Then click on additional actions and configure, then click on code organizing, select remove trailing lines and all lines. After that, you should remove the trailing lines from: XenServerStorageMotionStrategy. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Removed unnecessary @Local annotations an...
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/853#issuecomment-141711752 Hi @critofolini, I think that there is a problem in your PR in class: SecurityGroupManagerImpl --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Removed the empty class 'com.cloud.deploy...
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/856#issuecomment-141712362 @pedro-martins that is good that you removed and unused method in com.cloud.deploy.DeploymentPlanner, however you forgot to remove all of the implementations of classes that implement it. BareMetalPlanner is one of the classes that you forgot to fix, there are others --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Fixed return type Void to void in DataMot...
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/854#issuecomment-141713041 @rodrigo93 nice commit. Could you squash your commits? Here is a link, if you do not know how to do that. http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-8805: Domains become inactive ...
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/775#issuecomment-141720864 @nitin-maharana, you do not need to open a new PR. You can continue working on this one, just do whatever is needed and commit to this branch. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Removal of DefaultUserAuthenticator empty...
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/852#issuecomment-141721291 Nice work @alexandrelimassantana, the class âDefaultUserAuthenticatorâ does not bring anything new to the hierarchy. Moreover, it has a comment âUse this UserAuthenticator if users are already authenticated outsideâ, but it is an abstract class, there is no way to use that class the way it is. That class seems something that was left behind a long time ago. I would just add, could you remove the @Local annotations on classes that you touched? They are not needed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Removal of class AgentBasedStandaloneCons...
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/855#issuecomment-141721624 Hi @alexandrelimassantana, I checked and also could not find any references for that class. That is nice that you are removing some code that was left behind. It seems that something happened whit Jenkins while processing your PR. Could you just force push to run Jenkins analysis again? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
