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