You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by ahgittin <gi...@git.apache.org> on 2014/11/15 03:22:23 UTC

[GitHub] incubator-brooklyn pull request: minor fixes to catalog and jcloud...

GitHub user ahgittin opened a pull request:

    https://github.com/apache/incubator-brooklyn/pull/333

    minor fixes to catalog and jclouds properties

    

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

    $ git pull https://github.com/ahgittin/incubator-brooklyn persistence-can-export-and-more

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

    https://github.com/apache/incubator-brooklyn/pull/333.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 #333
    
----
commit 9ed1f9814c46b5347b9880fba0d106c156dd181f
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-11-14T23:58:24Z

    catalog js-gui tweaks
    
    * force server ordering to be respected on refresh, so new default versions show at root
    * update applications as well as entities
    * fix bug where sometimes on relead, policies would cause errors in browser

commit 9fd29505a8217863cd422d53d097fa0169cb35cd
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-11-15T00:46:09Z

    allow region and endpoint specified as explicit keys to override ones set implicitly

----


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

[GitHub] incubator-brooklyn pull request: minor fixes to catalog and jcloud...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/333#discussion_r20403381
  
    --- Diff: core/src/main/java/brooklyn/entity/rebind/dto/BasicEntityMemento.java ---
    @@ -198,11 +214,19 @@ protected BasicEntityMemento(Builder builder) {
         }
     
         protected AttributeSensor<?> getAttributeKey(String key) {
    +        AttributeSensor<?> result=null;
             if (attributeKeys!=null) {
    -            AttributeSensor<?> ak = attributeKeys.get(key);
    -            if (ak!=null) return ak;
    +            result = attributeKeys.get(key);
    +            if (result!=null && !LEGACY_KEY_DESCRIPTION.equals(result.getDescription()))
    +                return result;
             }
    -        return (AttributeSensor<?>) getStaticSensorKeys().get(key);
    +        AttributeSensor<?> resultStatic = (AttributeSensor<?>) getStaticSensorKeys().get(key);
    +        if (resultStatic!=null) return resultStatic;
    +        // if it was a legacy key, if it is added back, drop the legacy reference
    +        if (result!=null) return result;
    +        // can happen on rebind if a key has gone away; it will not be declared in the file or in the 
    --- End diff --
    
    Same as getConfigKey


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

[GitHub] incubator-brooklyn pull request: minor fixes to catalog and jcloud...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-brooklyn/pull/333


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

[GitHub] incubator-brooklyn pull request: minor fixes to catalog and jcloud...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/333#discussion_r20403373
  
    --- Diff: core/src/main/java/brooklyn/entity/rebind/dto/BasicEntityMemento.java ---
    @@ -180,12 +186,22 @@ protected BasicEntityMemento(Builder builder) {
             return staticConfigKeys;
         }
     
    +    final static String LEGACY_KEY_DESCRIPTION = "This item was defined in a different version of this blueprint; metadata unavailable here.";
    +    
         protected ConfigKey<?> getConfigKey(String key) {
    +        ConfigKey<?> result = null;
             if (configKeys!=null) {
    -            ConfigKey<?> ck = configKeys.get(key);
    -            if (ck!=null) return ck;
    +            result = configKeys.get(key);
    +            if (result!=null && !LEGACY_KEY_DESCRIPTION.equals(result.getDescription()))
    +                    return result;
             }
    -        return getStaticConfigKeys().get(key);
    +        ConfigKey<?> resultStatic = getStaticConfigKeys().get(key);
    +        if (resultStatic!=null) return resultStatic;
    +        // if it was a legacy key, if it is added back, drop the legacy reference
    --- End diff --
    
    How is the reference dropped if we return it? Is the comment for the above two lines?


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

[GitHub] incubator-brooklyn pull request: minor fixes to catalog and jcloud...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/333#discussion_r20403267
  
    --- Diff: core/src/main/java/brooklyn/location/basic/Locations.java ---
    @@ -111,6 +111,8 @@ public static void manage(Location loc, ManagementContext managementContext) {
         }
     
         public static Location coerce(ManagementContext mgmt, Object rawO) {
    +        if (rawO==null)
    --- End diff --
    
    With this fix the null check in the caller is not needed any more.


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

[GitHub] incubator-brooklyn pull request: minor fixes to catalog and jcloud...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/333#discussion_r20403376
  
    --- Diff: core/src/main/java/brooklyn/entity/rebind/dto/BasicEntityMemento.java ---
    @@ -180,12 +186,22 @@ protected BasicEntityMemento(Builder builder) {
             return staticConfigKeys;
         }
     
    +    final static String LEGACY_KEY_DESCRIPTION = "This item was defined in a different version of this blueprint; metadata unavailable here.";
    +    
         protected ConfigKey<?> getConfigKey(String key) {
    +        ConfigKey<?> result = null;
             if (configKeys!=null) {
    -            ConfigKey<?> ck = configKeys.get(key);
    -            if (ck!=null) return ck;
    +            result = configKeys.get(key);
    +            if (result!=null && !LEGACY_KEY_DESCRIPTION.equals(result.getDescription()))
    +                    return result;
             }
    -        return getStaticConfigKeys().get(key);
    +        ConfigKey<?> resultStatic = getStaticConfigKeys().get(key);
    +        if (resultStatic!=null) return resultStatic;
    +        // if it was a legacy key, if it is added back, drop the legacy reference
    +        if (result!=null) return result;
    +        // can happen on rebind if a key has gone away; it will not be declared in the file or in the 
    --- End diff --
    
    Unfinished sentence.


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

[GitHub] incubator-brooklyn pull request: minor fixes to catalog and jcloud...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/333#discussion_r20403279
  
    --- Diff: locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocationResolver.java ---
    @@ -177,17 +177,20 @@ public JcloudsLocation newLocationFromString(Map locationFlags, String spec, bro
             Map jcloudsProperties = new JcloudsPropertiesFromBrooklynProperties().getJcloudsProperties(providerOrApi, regionOrEndpoint, namedLocation, allProperties);
             jcloudsProperties.putAll(locationFlags);
             
    -        if (isProvider) {
    -            // providers from ServiceLoader take a location (endpoint already configured), and optionally a region name
    -            // NB blank might be supplied if spec string is "mycloud:" -- that should be respected, 
    -            // whereas no parameter/regionName ie null value -- "mycloud" -- means don't set
    -            if (regionOrEndpoint!=null)
    -                jcloudsProperties.put(JcloudsLocationConfig.CLOUD_REGION_ID.getName(), regionOrEndpoint);
    -        } else {
    -            // other "providers" are APIs so take an _endpoint_ (but not a location);
    -            // see note above re null here
    -            if (regionOrEndpoint!=null)
    -                jcloudsProperties.put(JcloudsLocationConfig.CLOUD_ENDPOINT.getName(), regionOrEndpoint);
    +        if (regionOrEndpoint!=null) {
    +            // apply the regionOrEndpoint (e.g. from the parameter) as appropriate -- but only if it has not been overridden
    +            if (isProvider) {
    +                // providers from ServiceLoader take a location (endpoint already configured), and optionally a region name
    +                // NB blank might be supplied if spec string is "mycloud:" -- that should be respected, 
    +                // whereas no parameter/regionName ie null value -- "mycloud" -- means don't set
    +                if (regionOrEndpoint!=null && Strings.isBlank(Strings.toString(jcloudsProperties.get(JcloudsLocationConfig.CLOUD_REGION_ID.getName()))))
    +                    jcloudsProperties.put(JcloudsLocationConfig.CLOUD_REGION_ID.getName(), regionOrEndpoint);
    +            } else {
    +                // other "providers" are APIs so take an _endpoint_ (but not a location);
    +                // see note above re null here
    +                if (regionOrEndpoint!=null && Strings.isBlank(Strings.toString(jcloudsProperties.get(JcloudsLocationConfig.CLOUD_ENDPOINT.getName()))))
    --- End diff --
    
    != null not needed as above.


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

[GitHub] incubator-brooklyn pull request: minor fixes to catalog and jcloud...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/333#issuecomment-63178569
  
    LGTM!


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

[GitHub] incubator-brooklyn pull request: minor fixes to catalog and jcloud...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/333#discussion_r20403272
  
    --- Diff: locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocationResolver.java ---
    @@ -177,17 +177,20 @@ public JcloudsLocation newLocationFromString(Map locationFlags, String spec, bro
             Map jcloudsProperties = new JcloudsPropertiesFromBrooklynProperties().getJcloudsProperties(providerOrApi, regionOrEndpoint, namedLocation, allProperties);
             jcloudsProperties.putAll(locationFlags);
             
    -        if (isProvider) {
    -            // providers from ServiceLoader take a location (endpoint already configured), and optionally a region name
    -            // NB blank might be supplied if spec string is "mycloud:" -- that should be respected, 
    -            // whereas no parameter/regionName ie null value -- "mycloud" -- means don't set
    -            if (regionOrEndpoint!=null)
    -                jcloudsProperties.put(JcloudsLocationConfig.CLOUD_REGION_ID.getName(), regionOrEndpoint);
    -        } else {
    -            // other "providers" are APIs so take an _endpoint_ (but not a location);
    -            // see note above re null here
    -            if (regionOrEndpoint!=null)
    -                jcloudsProperties.put(JcloudsLocationConfig.CLOUD_ENDPOINT.getName(), regionOrEndpoint);
    +        if (regionOrEndpoint!=null) {
    +            // apply the regionOrEndpoint (e.g. from the parameter) as appropriate -- but only if it has not been overridden
    +            if (isProvider) {
    +                // providers from ServiceLoader take a location (endpoint already configured), and optionally a region name
    +                // NB blank might be supplied if spec string is "mycloud:" -- that should be respected, 
    +                // whereas no parameter/regionName ie null value -- "mycloud" -- means don't set
    +                if (regionOrEndpoint!=null && Strings.isBlank(Strings.toString(jcloudsProperties.get(JcloudsLocationConfig.CLOUD_REGION_ID.getName()))))
    --- End diff --
    
    Already checked non-null regionOrEndpoint above.


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