You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Andrea Turli <no...@github.com> on 2017/08/06 15:02:55 UTC
[jclouds/jclouds] JCLOUDS-1323: use security group names in openstack
nova template options (#1128)
This patch is to validate the `templateOption.securityGroups` passed by the user. It accepts `securityGroupNames` as it uses to be before https://github.com/jclouds/jclouds/pull/1117 cc @neykov @nacx
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds/pull/1128
-- Commit Summary --
* JCLOUDS-1323: use security group names in openstack nova template options
-- File Changes --
M apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/strategy/ApplyNovaTemplateOptionsCreateNodesWithGroupEncodedIntoNameThenAddToSet.java (17)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/1128.patch
https://github.com/jclouds/jclouds/pull/1128.diff
--
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
Re: [jclouds/jclouds] JCLOUDS-1323: use security group names in
openstack nova template options (#1128)
Posted by Ignasi Barrera <no...@github.com>.
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
Re: [jclouds/jclouds] JCLOUDS-1323: use security group names in
openstack nova template options (#1128)
Posted by Andrea Turli <no...@github.com>.
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)
Posted by Andrea Turli <no...@github.com>.
@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)
Posted by Svet <no...@github.com>.
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)
Posted by Andrea Turli <no...@github.com>.
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)
Posted by Svet <no...@github.com>.
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)
Posted by Andrea Turli <no...@github.com>.
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)
Posted by Andrea Turli <no...@github.com>.
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)
Posted by Andrea Turli <no...@github.com>.
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