[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-03-31 Thread nvazquez
GitHub user nvazquez opened a pull request: https://github.com/apache/cloudstack/pull/1457 CLOUDSTACK-9333: Exclude clusters from OVF operations JIRA TICKET: https://issues.apache.org/jira/browse/CLOUDSTACK-9333 It is proposed to add a way to exclude hosts from selected clu

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-03-31 Thread rafaelweingartner
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-204147535 @nvazquez after reading your explanation at the PR's body I understood the change. That change can be used to any kind of hypervisor, right?

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-03-31 Thread serg38
Github user serg38 commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-204152844 @rafaelweingartner How about 'cluster.exclude' or 'cluster.storage.operations.exclude'. I think the latter is the better choice. --- If your project is set up for i

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-03-31 Thread rafaelweingartner
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-204153083 @serg38, I agree with you. The name 'cluster.storage.operations.exclude' seems to suit better the parameter. --- If your project is set up for it, yo

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-03-31 Thread rafaelweingartner
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-204197434 @nvazquez I was thinking, isn't there any documentation/enumeration for all of the possible parameters to be used at the "cluster_details" table?

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-04-01 Thread nvazquez
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-204431280 @rafaelweingartner thanks for reviewing! I couldn't find any document with all cluster_details, however we could find out that this property could be defined like

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-04-01 Thread rafaelweingartner
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-204624408 @nvazquez, I find the “AlertManager” class terrible for that. Its name gives the idea of something that manages alerts and not clusters parameters

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-04-04 Thread nvazquez
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-205316122 @rafaelweingartner Ok, nice. I like that option. The only question I have about this is that if property could be changed from UI by users after inserted in "

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-04-04 Thread rafaelweingartner
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-205334162 I have just tested; the cluster_details table does not work as the configuration table in which you can add an entry there and then that parameter is disp

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-04-04 Thread rafaelweingartner
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-205349316 @nvazquez just an update. I was going over "org.apache.cloudstack.api.command.admin.config.ListCfgsByCmd.execute()" that is used to list the cluster c

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-04-04 Thread nvazquez
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-205409749 @serg38 and I found out a way to change this property from UI, inserting the property in "configuration" table with scope="Cluster", insert sql: INSERT INT

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-04-04 Thread nvazquez
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-205414291 I pushed a solution with this changes, without including insert in sql schema, do you think this way could be ok? --- If your project is set up for it, you can re

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-04-04 Thread rafaelweingartner
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-205423791 I think this is the best way to tackle the problem. You just need to add that insert into the version’s upgrade script. --- If your project is set

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-04-05 Thread nvazquez
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-205857772 @rafaelweingartner cool! I added it. --- 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 projec

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-04-05 Thread rafaelweingartner
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-205882042 For me everything (with the code) is ok now, LGTM for the PR. Could you please run functional tests? --- If your project is set up for it, you can re

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-04-05 Thread swill
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-205883002 @rafaelweingartner I can run tests via KVM, but I don't have the ability to run tests against VMware hardware. Are tests against KVM good enough in this case? ---

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-04-05 Thread rafaelweingartner
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-205885858 Good question, those changes affect every single hypervisor. I have reviewed the code and after that I can say that if those changes work for KVM, I s

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-04-05 Thread swill
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-205892175 Perfect. Thank you both for verifying this. I probably won't be able to get to testing this till tomorrow evening because I am about to reinstall my CI to get my te

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-04-05 Thread nvazquez
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-205917619 thanks @rafaelweingartner and @swill ! --- 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 proj

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-04-07 Thread swill
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-206921614 ## CI RESULTS **84/85 TESTS PASSED** The only failed test is a test that often fails in my environment and is unrelated to t

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-04-07 Thread nvazquez
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-206925844 Looks nice! Thanks @swill for testing it in your environment! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHu

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-04-07 Thread swill
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-206958139 Based on @rafaelweingartner's code review and the CI results, I think this is ready to merge. --- If your project is set up for it, you can reply to this email and

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-04-07 Thread rafaelweingartner
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-206969279 @swill, I was going to say the same. I also believe that this PR is ready to be merged. But, I would like to have heard some other committer feedback.

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-04-07 Thread swill
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-206978473 @rafaelweingartner Ya, ideally I would like to try to get two code reviews for each PR. I try to do a quick code review on each, but with the number for PRs I am try

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-04-07 Thread rafaelweingartner
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-206980344 @swill you are doing a marvelous job; I totally understand your workload. I hope we can get more people to help us reviewing PRs; that number has been

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-04-07 Thread serg38
Github user serg38 commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-206989623 @swill , @rafaelweingartner I work very closely with @nvazquez. His PRs pass very rigorous testing in our Lab on physical Vmware hypervisors. I reviewed his code and

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-04-07 Thread serg38
Github user serg38 commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-206990689 @rafaelweingartner On a general note we got a verbal commitment from Accelerite who bought CloudPlatform from Citrix last month to contribute more to the community

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-04-07 Thread rafaelweingartner
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-206992288 @serg38 thats is great. So, Do I or you (@Swill) do the honors to execute the merge. --- If your project is set up for it, you can reply to this email

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-04-07 Thread swill
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1457#issuecomment-207014015 @serg38 awesome, thank you. I will get this merged... Yes, good new about Accelerite. I am very hopeful they will be able to add value to the ACS community

[GitHub] cloudstack pull request: CLOUDSTACK-9333: Exclude clusters from OV...

2016-04-07 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/1457 --- 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