brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ahgittin <...@git.apache.org>
Subject [GitHub] brooklyn-server pull request #853: [WIP] deprecate Task as values in config ...
Date Wed, 04 Oct 2017 15:17:33 GMT
GitHub user ahgittin opened a pull request:

    https://github.com/apache/brooklyn-server/pull/853

    [WIP] deprecate Task as values in config and getImmediately(Task) in favour of TaskFactory

    As discussed in #816 and suggested to be fixed in #835 but pushed out to here:
    
    There are bad things that can happen if an unsubmitted `Task` is set as a config value:
 attempts to read it change the behaviour for subsequent reads.  In particular a call to `getImmediately`
(which can be done during validation, or maybe even during a REST read) can cause it to be
submitted in an interrupted state, which damages it for all future use.  But more generally,
once it has been evaluated it will always and only return that value, where a user may sensibly
expect it to return the current value on each read.
    
    Changing those methods -- `config().set(key, task)` and `getImmediately(task)` -- to take
`TaskFactory` instances instead removes this confusion and problems, making it clear that
a new task is run on each execution.
    
    However this is a big job due to the prevalence of `Task` being used where a `TaskFactory`
is now wanted.  In particular `DependentConfiguration` returns `Task` instances!  (There is
a long-standing comment that `DslComponent` should be used instead, as that generates factories
internal, and YAML use behaves as proposed to implement here across the board, so it is mainly
a concern internally and for people using the Java API.  Still it is a clean-up worth doing.)
    
    I've committed one set of changes so far (it is just one commit beyond the merge of #835),
signposting the direction this will go and logging warnings in the deprecated code paths.
 Next I will actually deprecate these and update usages.
    
    In particular I will likely deprecate all `Task`-returning `DependentConfig` with `TaskFactory`
alternatives; then in a subsequent version we can switch the deprecated methods to return
`TaskFactory`:  this means code such as `config().set(key, attributeWhenReady(...))` although
it will be deprecated for one release will be migrated with no effort from a user to the new
behaviour.
    
    Comments at this early stage very welcome cc @aledsage @sjcorbett @grkvlt .


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

    $ git pull https://github.com/ahgittin/brooklyn-server task-immediately-improvements

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

    https://github.com/apache/brooklyn-server/pull/853.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 #853
    
----

----


---

Mime
View raw message