andreaturli requested changes on this pull request.
thanks @neykov, this looks useful, I've just some comments
> @@ -191,14 +191,20 @@ public boolean removeSecurityGroup(String id) {
return false;
}
- if (sgApi.get().get(groupId) == null) {
- return false;
+ // Would be nice to delete the group and invalidate the cache atomically
- i.e. use a mutex.
+ // Will make sure that a create operation in parallel won't see
inconsistent state.
+
+ boolean deleted = sgApi.get().delete(groupId);
+
+ for (SecurityGroupInRegion cachedSg : groupCreator.asMap().values()) {
do you need to invalidate if `deleted` is `false` ?
> @@ -191,14 +191,20 @@ public boolean removeSecurityGroup(String id) {
return false;
}
- if (sgApi.get().get(groupId) == null) {
- return false;
+ // Would be nice to delete the group and invalidate the cache atomically
- i.e. use a mutex.
+ // Will make sure that a create operation in parallel won't see
inconsistent state.
+
+ boolean deleted = sgApi.get().delete(groupId);
what happens if the `groupId` doesn't exist?
--
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/1110#pullrequestreview-43102192
