Deprecates use of Task in ConfigKey #883
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Also avoid
StackOverflowError
inExecutionContext.getImmediately(task)
When a non-
BasicTask
is passed in.The motivation for deprecating setting a
ConfigKey
as aTask
is that we really shouldn't be doing it! It can get very messed up if one tries togetImmediately
or get non-blocking (e.g. by aTransformer
that is trying to evaluate its result when triggered). That can leave the task in a cancelled state, so all subsequent calls totask.get()
(i.e. all calls toconfig().get(mykey)
) will fail!Instead, we should be using
TaskFactory
(which is how the DSL evaluation works).It's good to deprecate this before a major release of 1.0.0.
It would be good to add an overloaded method for setting config to
set(ConfigKey<T>, TaskFactory<T>)
so that such usage doesn't involve ugly casting to work around the strongly typed generics. We can do that as a stage two, separate from this PR.