brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Heneveld <alex.henev...@cloudsoftcorp.com>
Subject Re: [PROPOSAL] $brooklyn:attributeWhenReady semantics
Date Thu, 11 Aug 2016 22:53:32 GMT

Andrew, Svet-

What's the actual boolean sensor that you want to respond to when it's 
false?  I'm curious whether a better answer lies in changing the 
semantics of the sensors specific to your blueprint.

Technically your solutions are good (and I like 1 (though probably 
taking a string or map to attribute when ready, not a list), + 2 and 3, 
and maybe 5 (but again not with the list)) -- but if we can recast in a 
natural to use default attributeWhenReady that's preferable and lowers 
the priority for this change.

Regarding 5 (and my DSL proposal) I'm kinda thinking -- for the longer 
term -- we have a way to register "predicate" as a type in brooklyn with 
a custom {de,}serializer as part of its definition that understands 
things like "= 42" then we'd have an extensible and self-documenting way 
to support such DSLs.  I'd prefer not do anything ad hoc in that 
direction if we don't have to.

--A


On 11/08/2016 17:58, Andrew Kennedy wrote:
> Yes, what Svet said is exactly right. I want to know the status of a
> boolean sensor which can usefully be set to false. I think the behaviour is
> currently is rather surprising - well, it surprised me when I realised my
> blueprint didn't work because I was waiting on a false value!
>
> I'd be happy with adding extra arguments to `attributeWhenReady` so that we
> could retain backward compatibility. Something like the following:
>
> 1. These are equivalent, and work the same way as the existing DSL function
> (see first example in the list)
>
>      $brooklyn:entity("whatever").attributeWhenReady("service.isUp")
>
>      $brooklyn:entity("whatever").attributeWhenReady:
>        - $brooklyn:sensor("service.isUp")
>
>      $brooklyn:entity("whatever").attributeWhenReady:
>        - service.isUp
>
>      $brooklyn:entity("whatever").attributeWhenReady:
>        - sensor: service.isUp
>
>      $brooklyn:entity("whatever").attributeWhenReady:
>        - sensor: $brooklyn:sensor("service.isUp")
>
> 2. This uses the new mechanism in
> https://github.com/apache/brooklyn-server/pull/291 to create an object from
> a factory method that provides the readiness predicate
>
>      $brooklyn:entity("whatever").attributeWhenReady:
>        - sensor: $brooklyn:sensor("cluster.first")
>          ready:
>            $brooklyn:object:
>              - type: org.apache.brooklyn.predicates.EntityPredicates
>                factoryMethod.name: equalTo
>                factoryMethod.args: true
>
> 3. This uses existing $brooklyn:object syntax to create a readiness
> predicate from a known classname that has a no-arg ctor
>
>      $brooklyn:entity("whatever").attributeWhenReady:
>        - sensor: $brooklyn:sensor("cluster.first")
>          ready:
>            $brooklyn:object:
>              - type: org.apache.brooklyn.predicates.EntityPredicates.NotNull
>
>      $brooklyn:entity("whatever").attributeWhenReady:
>        - sensor: $brooklyn:sensor("cluster.first")
>          ready:
>            $brooklyn:object:
>              - type: org.apache.brooklyn.predicates.EntityPredicates.EqualTo
>                object.fields:
>                  equalTo: true
>
> 4. Here we use a new DSL function that understands how to create predicate
> objects
>
>      $brooklyn:entity("whatever").attributeWhenReady:
>        - sensor: $brooklyn:sensor("cluster.first")
>          ready: $brooklyn:predicate:equalTo(true)
>
> 5. Here we allows string constants to describe some well known predicates
> that would be looked up from an internal list
>
>      $brooklyn:entity("whatever").attributeWhenReady:
>        - sensor: $brooklyn:sensor("cluster.first")
>          ready: "notNul"l
>
>      $brooklyn:entity("whatever").attributeWhenReady:
>        - sensor: $brooklyn:sensor("cluster.first")
>          ready: "isTrue"
>
>      $brooklyn:entity("whatever").attributeWhenReady:
>        - sensor: $brooklyn:sensor("cluster.first")
>          ready: "immediate"
>
> As you can see, the new functionality we want will require some changes to
> the DSL no matter what, although #291 is promising in terms of allowing us
> to create predicates on the fly in the DSL. I'm not sure about an explicit
> `$brooklyn:predicate` addition, it seems far too specific. I think the
> current functionality is too restrictive though, since it would require a
> new class for every predicate, structured as a Java bean with appropriate
> setters. One solution is to use the mechanism employed by enrichers, where
> they accept strings indicating the function to be applied, "average", "sum"
> and so on, and allow "notNull" or "immediate" as an argument for the
> readiness predicate.
>
> I'm not sure about overloading this into a description of the expression,
> as Alex suggests. If we start to allow "= false", "> 4", "equalTo
> 'RUNNING'" and so on then we will probably end up writing a parser for some
> limited DSL grammar for the predicate clauses, and this doesn't seem to be
> in the spirit of the blueprint language. If we want that, why not go the
> JSR-223 route, and accept embedded code as the predicate declaration:
>
> 6. Use JSR-223 to embed the code for the predicate
>
>      $brooklyn:entity("whatever").attributeWhenReady:
>        - sensor: $brooklyn:sensor("service.state")
>          ready.lang: "JavaScript"
>          ready.code: |
>            function accept(input) {
>              if (input == "RUNNING") return true;
>              if (input.startsWith("STOP") return true;
>              return false;
>            }
>
> But, I think 2., 3. and 5. above are the way to go, assuming #291 is merged
> and we can use its new syntax for objects.
>
> Cheers,
> Andrew.
>
> On Mon, 8 Aug 2016 at 09:56 Svetoslav Neykov <
> svetoslav.neykov@cloudsoftcorp.com> wrote:
>
>> Initial reason for the change is doing "attributeWhenReady" on boolean
>> sensor with "false" as the expected value. That blocks indefinitely with
>> the current implementation.
>>
>> Using an enricher wouldn't allow for customizing the timeout - currently
>> "indefinite" when starting and 1 min when stopping. We could extend
>> "attributeWhenReady" with a timeout argument only (if needed) and defer the
>> rest to an enricher.
>> As for "option 2" we could fix it in a backwards compatible way:
>>    * introduce "attributeWhenTruthy", alias to "attributeWhenReady"
>>    * introduce "attributeWhenPresent" with a notNull && notEmpty check
>>    * deprecate "attributeWhenReady"
>>
>> Svet.
>>
>>
>>> On 7.08.2016 г., at 15:03, Alex Heneveld <
>> alex.heneveld@cloudsoftcorp.com> wrote:
>>> You could add an isSet enricher then wait on it. I prefer customisation
>> in
>>> enrichers rather than extending the dsl.
>>>
>>> What is the use case? Everywhere I've seen whenReady is the most natural.
>>>
>>> Best
>>> Alex
>>>
>>> On 5 Aug 2016 17:07, "Aled Sage" <aled.sage@gmail.com> wrote:
>>>
>>>> Hi all,
>>>>
>>>> Motivated by Andrew's recent PR [1] and Svet's suggestion, I'd like to
>>>> propose a change to the semantics/arguments to
>>>> $brooklyn:attributeWhenReady().
>>>>
>>>> _*Current Behaviour*_
>>>>
>>>> _Simple example_
>>>> If you write in a YAML blueprint something like:
>>>>
>>>>    brooklyn.config:
>>>>       myurl:
>> $brooklyn:component("db").attributeWhenReady("datastore.url")
>>>> then when you attempt to get the config value for "myurl" it will block
>>>> until the "datastore.url" sensor value is available on the entity with
>> id
>>>> "db".
>>>>
>>>> _"Groovy truth" example_
>>>> The attributeWhenReady actually blocks until groovy truth is satisfied
>>>> (boolean is true, collection is non-empty, string is non-empty, etc).
>>>>
>>>> You can therefore write:
>>>>
>>>>    brooklyn.config:
>>>>       launch.latch:
>>>>    $brooklyn:component("db").attributeWhenReady("service.isUp")
>>>>
>>>> and it will block until the db entity's service.isUp sensor is true.
>>>>
>>>> Unfortunately this means it's impossible to wait for a boolean attribute
>>>> to be non-null (i.e. stop waiting when false or true).
>>>>
>>>> _Java API Functionality_
>>>> Comparing the yaml DSL with what can be done in DependentConfiguration
>>>> [2], there is a lot we can't do:
>>>>
>>>> * customize the "readiness" function (i.e. what sensor values mean
>>>>    "ready")
>>>> * define "abort conditions" (e.g. stop waiting if the entity
>>>>    transitions to some other state)
>>>> * specify a timeout
>>>> * give a default value for if there is a timeout, or if the entity is
>>>>    removed while we're waiting.
>>>>
>>>>
>>>> _*Proposal: option 1*_
>>>>
>>>> My favoured option is to make attributeWhenReady more configurable.
>>>>
>>>> For example, we could support:
>>>>
>>>>    brooklyn.config:
>>>>       launch.latch: $brooklyn:component("db").attributeWhenReady:
>>>>         sensor: "service.isUp"
>>>>         readiness: $brooklyn:predicates.equalTo("true")
>>>>         timeout: 10m
>>>>
>>>> Maybe we'd support overloading of in-lining the args (relying on
>> parameter
>>>> order of [sensor, readiness, timeout]), such as:
>>>>
>>>>    brooklyn.config:
>>>>       launch.latch:
>>>>    $brooklyn:component("db").attributeWhenReady("service.isUp",
>>>>    $brooklyn:predicates.equalTo("true"), 10m)
>>>>
>>>> I think I'm ok with leaving the default as groovy-truth semantics, as
>> long
>>>> as we make the documentation clearer for that.
>>>>
>>>> However, this proposal might well be hard to implement. It would likely
>>>> require significant change to how we parse the $brooklyn expressions
>> (if my
>>>> recollection of this code is correct).
>>>>
>>>> _*Proposal: option 2*_
>>>>
>>>> An alternative suggested by Svet in a comment on [1]:
>>>>
>>>>    Should we fix attributeWhenReady instead? It's confusing to have it
>>>>    wait indefinitely on false.
>>>>    Having another attribute, very similar to attributeWhenReady
>>>>    complicates things even further.
>>>>
>>>>    Suggest renaming attributeWhenRedy to attributeWhenTruth and
>>>>    changing attributeWhenReady to wait on notNull && notEmpty.
>>>>
>>>> That sounds like a good idea - it feels much more explicit, and is very
>>>> simple to implement.
>>>>
>>>> However, it would break backwards compatibility (e.g. for those using
>>>> `attributeWhenReady("service.isUp")`.
>>>>
>>>> If we think the semantics are wrong, then we should definitely change it
>>>> while we're still on version 0.x rather than ignoring it.
>>>>
>>>> Aled
>>>>
>>>> [1] https://github.com/apache/brooklyn-server/pull/282
>>>> [2] https://github.com/apache/brooklyn-server/blob/rel/apache-
>>>> brooklyn-0.9.0/core/src/main/java/org/apache/brooklyn/core/
>>>> sensor/DependentConfiguration.java#L669
>>>>
>>>>
>> --
> Andrew Kennedy ; Founder clocker.io project ; @grkvlt ; Cloudsoft
>


Mime
View raw message