osdir.com


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[GitHub] brooklyn-server issue #971: DSL for $brooklyn:location()


Github user aledsage commented on the issue:

    https://github.com/apache/brooklyn-server/pull/971
  
    Thanks @geomacy @nakomis 
    
    I've added `testDslLocationIndexOutOfBounds`, which asserts that we get a nice exception when one tries to retrieve in anger the config value that has an invalid index.
    
    I think this is tricky to improve, without going into bigger more general changes.
    
    As you point out, for the blueprint:
    ```
    location: localhost
    services:
      - type: org.apache.brooklyn.entity.stock.BasicApplication
        brooklyn.config:
          config1: $brooklyn:location()
          config2: $brooklyn:location("0")
          config3: $brooklyn:location("1")
          config4: $brooklyn:attributeWhenReady("sensorDoesNotExist")
    ```
    
    `br app obrjk09hec config` returns:
    ```
    Key                  | Value   
    config1              | map[type:org.apache.brooklyn.api.location.Location id:whoq9008c0]   
    defaultDisplayName   | <nil>   
    camp.template.id     | ClysL4Gs   
    config2              | map[type:org.apache.brooklyn.api.location.Location id:whoq9008c0]   
    config3              | map[component:map[componentId: componentIdSupplier:<nil> scopeComponent:<nil> scope:THIS] index:1]   
    config4              | map[component:map[componentId: componentIdSupplier:<nil> scopeComponent:<nil> scope:THIS] sensorName:sensorDoesNotExist]   
    
    ```
    
    The `config3` evaluation has only been logged at debug, because the value isn't yet being used - it's just retrieved for display purposes, so hasn't blocked. It failed to resolve immediately, so we go the DSL object back. You can see we get the same behaviour for an `attributeWhenReady` that cannot yet be evaluated.
    
    If we wrote a blueprint that actually tried to use it (i.e. that caused `entity.config().get("config3")` to be called), then it would throw the exception, which would be logged at warn.
    
    The two things you point out would be good more general improvements:
    
    1. `/v1/applications/<appId>/entities/<entityId>/config/current-state?raw=false` should return nicer representation of DSL objects (e.g. calling `toString()` on it, rather than showing the values of the fields `component` and `index`).
    
    2. Location representation in the above call doesn't give much info - just the id, and that it's a location. It would be good to give more info as well (e.g. the display name).
    
    ---
    Are you ok with merging this PR as-is, and us making those more general improvements separately?


---