brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From neykov <...@git.apache.org>
Subject [GitHub] brooklyn-server pull request #425: Factor security group code out into separ...
Date Wed, 23 Nov 2016 12:01:57 GMT
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/425#discussion_r89302475
  
    --- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java
---
    @@ -223,56 +230,64 @@ private SecurityGroup getSecurityGroup(final String nodeId, final
SecurityGroupE
          * @param location Location to gain permissions
          * @param permissions The set of permissions to be applied to the location
          */
    -    public JcloudsLocationSecurityGroupCustomizer addPermissionsToLocation(final JcloudsMachineLocation
location, final Iterable<IpPermission> permissions) {
    -        ComputeService computeService = location.getParent().getComputeService();
    -        addPermissionsToLocationAndReturnSecurityGroup(computeService, location, permissions);
    +    public JcloudsLocationSecurityGroupCustomizer addPermissionsToLocation(final JcloudsMachineLocation
location,
    +              final Iterable<IpPermission> permissions) {
    +        addPermissionsToLocationAndReturnSecurityGroup(location, permissions);
             return this;
         }
     
    -    public Collection<SecurityGroup> addPermissionsToLocationAndReturnSecurityGroup(ComputeService
computeService, final JcloudsMachineLocation location, final Iterable<IpPermission>
permissions) {
    +    public Collection<SecurityGroup> addPermissionsToLocationAndReturnSecurityGroup(
    +        final JcloudsMachineLocation location, final Iterable<IpPermission> permissions)
{
    +
             synchronized (JcloudsLocationSecurityGroupCustomizer.class) {
    --- End diff --
    
    I'm asking because of [this](https://github.com/apache/brooklyn-server/blob/master/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java#L217-L221)
comment. Looks like the same restrictions will apply on the synchronizer.
    Agree it can be tackled in a separate PR.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message