Re: [jclouds/jclouds] JCLOUDS-1323: use security group names in openstack nova template options (#1128)
Closed #1128. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1128#event-1198830638
Re: [jclouds/jclouds] JCLOUDS-1323: use security group names in openstack nova template options (#1128)
merged at [master](http://git-wip-us.apache.org/repos/asf/jclouds/commit/7c58f9d7) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1128#issuecomment-321216723
Re: [jclouds/jclouds] JCLOUDS-1323: use security group names in openstack nova template options (#1128)
thanks @neykov, merging now -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1128#issuecomment-321200716
Re: [jclouds/jclouds] JCLOUDS-1323: use security group names in openstack nova template options (#1128)
neykov approved this pull request. :+1: -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1128#pullrequestreview-55171544
Re: [jclouds/jclouds] JCLOUDS-1323: use security group names in openstack nova template options (#1128)
ok thanks guys, I've fixed an expected test, good to merge now? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1128#issuecomment-321174304
Re: [jclouds/jclouds] JCLOUDS-1323: use security group names in openstack nova template options (#1128)
@andreaturli pushed 1 commit. 9d7f729 fix NovaComputeServiceExpectTest -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/jclouds/jclouds/pull/1128/files/69ded18e0f13f1b77d04d8d83225930561b11330..9d7f729be91d0973352ccbf758ccc30a38612001
Re: [jclouds/jclouds] JCLOUDS-1323: use security group names in openstack nova template options (#1128)
andreaturli commented on this pull request.
> for (String securityGroupName : templateOptions.getGroups()) {
-
checkNotNull(novaApi.getSecurityGroupApi(region).get().get(securityGroupName),
"security group %s doesn't exist", securityGroupName);
+checkState(securityGroupNames.contains(securityGroupName), "Cannot
find security group with name " + securityGroupName + ". \nSecurity groups
available are: \n" + Iterables.toString(securityGroupNames)); // {
}
@neykov yes that was the idea
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1128#discussion_r131727863
Re: [jclouds/jclouds] JCLOUDS-1323: use security group names in openstack nova template options (#1128)
neykov commented on this pull request.
> for (String securityGroupName : templateOptions.getGroups()) {
-
checkNotNull(novaApi.getSecurityGroupApi(region).get().get(securityGroupName),
"security group %s doesn't exist", securityGroupName);
+checkState(securityGroupNames.contains(securityGroupName), "Cannot
find security group with name " + securityGroupName + ". \nSecurity groups
available are: \n" + Iterables.toString(securityGroupNames)); // {
}
Wouldn't give as nice error message as the above one - not clear which security
group is missing with your suggestion.
LGTM overall.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1128#discussion_r131720327
Re: [jclouds/jclouds] JCLOUDS-1323: use security group names in openstack nova template options (#1128)
nacx approved this pull request.
> for (String securityGroupName : templateOptions.getGroups()) {
-
checkNotNull(novaApi.getSecurityGroupApi(region).get().get(securityGroupName),
"security group %s doesn't exist", securityGroupName);
+checkState(securityGroupNames.contains(securityGroupName), "Cannot
find security group with name " + securityGroupName + ". \nSecurity groups
available are: \n" + Iterables.toString(securityGroupNames)); // {
}
[minor] Change to `securityGroupNames.containsAll(templateOptions.getGroups())`.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1128#pullrequestreview-54640480
