[GitHub] cloudstack pull request: CLOUDSTACK-8897: baremetal:addHost:make h...

2016-05-02 Thread rhtyd
Github user rhtyd commented on the pull request:

https://github.com/apache/cloudstack/pull/874#issuecomment-216193842
  
@harikrishna-patnala please rebase against latest master and push -f, 
update on status of your PR

LGTM
tag:easypr


---
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-8897: baremetal:addHost:make h...

2016-02-14 Thread alexandrelimassantana
Github user alexandrelimassantana commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/874#discussion_r52846753
  
--- Diff: server/src/com/cloud/resource/ResourceManagerImpl.java ---
@@ -680,6 +680,13 @@ public Discoverer getMatchingDiscover(final 
Hypervisor.HypervisorType hypervisor
 }
 }
 
+if 
((hypervisorType.equalsIgnoreCase(HypervisorType.BareMetal.toString( {
+if (hostTags == null || hostTags.size() == 0) {
--- End diff --

Hey @harikrishna-patnala just a little tip on line 684. You could use 
Apache Commons' CollectionUtils to make such checks, it does exactly the same 
thing but is a little more clean. But that's just a little adjustment. 
http://commons.apache.org/proper/commons-collections/javadocs/api-release/org/apache/commons/collections4/CollectionUtils.html#isEmpty(java.util.Collection)
 


---
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-8897: baremetal:addHost:make h...

2015-09-30 Thread runseb
Github user runseb commented on the pull request:

https://github.com/apache/cloudstack/pull/874#issuecomment-144332355
  
We won't be able to test this on simulator.
@harikrishna-patnala can you answer @borisroman and then we can merge.
LGTM +1 on code review alone


---
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-8897: baremetal:addHost:make h...

2015-09-22 Thread harikrishna-patnala
GitHub user harikrishna-patnala opened a pull request:

https://github.com/apache/cloudstack/pull/874

CLOUDSTACK-8897: baremetal:addHost:make host tag info mandtory in bar…

CLOUDSTACK-8897: baremetal:addHost:make host tag info mandtory in baremetal 
addhost Api call

addhost api is successful with out providing the host tag info and we 
recommend host tag is mandatory for bare-metal.
In the current implementation host tag check is happening at vm deployment 
stage but it will be good to have host tag field as mandatory field during 
adding of the host 

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/harikrishna-patnala/cloudstack CLOUDSTACK-8897

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/874.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 #874


commit 88f023cee3f2a52c3d1f6452cac84aa03477a2f1
Author: Harikrishna Patnala 
Date:   2015-09-22T11:41:16Z

CLOUDSTACK-8897: baremetal:addHost:make host tag info mandtory in baremetal 
addhost Api call

addhost api is successful with out providing the host tag info and we 
recommend host tag is mandatory for bare-metal.
In the current implementation host tag check is happening at vm deployment 
stage but it will be good to have host tag field as mandatory field during 
adding of the host it self.




---
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.
---