Hi all,
TL;DR: deprecate `@SetFromFlag`; instead explicitly set
deprecatedAlias (and/or alias?) on the ConfigKey.
We should change this *after* releasing 0.10.0, to decrease risk.
_*Current Situation*_
When writing a Java entity, one can declare a config key type as a
static field. One can also use the annotation `@SetFromFlag` on this.
The annotation allows one to include an alternative name for the key.
This alternative name is often confusing. It is not part of the
formal
"config key" definition (i.e. not available via
entity.getEntityType()), and is only respected in some situations
when
supplying config values (e.g. when supplying config directly to an
entity, but not when using config inheritance).
The `@SetFromFlag` annotation also supports "defaultVal", but this is
(hopefully!) never used. There is a better way to specify a default
value, which is to pass it into the Config Key's definition.
The other options on the `@SetFromFlag` annotation are "immutable"
and
"nullable". These are ignored in some/many situations, so are (I
believe) a bad idea to use (certainly bad to rely on). It's better to
use `.constraint(Predicates.notNull())` on the config key, rather
than
"nullable".
The reason for supporting multiple names can be split into (at
least?)
three different use-cases (we'll discuss the second and third in a
separate email thread):
1. Backwards compatibility (e.g. because we already support two
names,
so need to keep doing that; or because we want to rename a config
key, such as correcting its spelling).
This use-case could be reworded as the need to support deprecated
names.
2. Aliases (i.e. a deliberate desire to support different names,
because those different names are seen as good).
3. Hints on blueprint validation in a composer (e.g.
"environment.variables not valid; did you mean env?")
_*Proposal*_
We should stop using `@SetFromFlag` on config keys entirely -
deprecating its use, and warning anywhere it's used.
Instead, we should add support for multiple names on the ConfigKey
definition itself (i.e. to the ConfigKey interface). This will
make it
part of the entity's type. This will make it easier to support in
yaml
blueprints, and also possible to deprecate config key names in YAML
brooklyn.parameters.
I suggest we support something like this:
ConfigKey<String> PRE_INSTALL_COMMAND =
ConfigKeys.builder(String.class, "my.toy.example")
.description("...")
.defaultVal("myDefaultVal")
.deprecatedAlias("my.toyy.example") // e.g. deprecated to correct
the misspelling
.deprecatedAliasSince("my.old.example", "0.10.0") // second
arg is
version when it was deprecated
.build();
Or in YAML:
brooklyn.parameters:
- name: my.toy.example
type: String
description: ...
default: myDefaultVal
deprecatedAliases:
- name: my.old.example
deprecatedSince: 0.10.0
# The version above is optional; it will also accept the
name
by itself:
- my.toyy.example
Here is a real-world example in Java (getting rid of
`@SetFromFlag("preInstallCommand")`):
ConfigKey<String> PRE_INSTALL_COMMAND =
ConfigKeys.builder(String.class, "pre.install.command")
.description("Command to be run prior to the install
method
being called on the driver")
.runtimeInheritance(BasicConfigInheritance.NOT_REINHERITED)
.deprecatedName("preInstallCommand")
.build();
When parsing the entity's Java config keys, we should be able to
translate the `@SetFromFlag` into an equivalent "deprecated aliases"
on the Config Key object (while preserving backwards compatibility).
When we can delete the `@SetFromFlag` support entirely, it will
really
clean up several areas of ugly/fiddly code!
==========
Part 2
==========
We should discuss *in a different thread* whether we want to support
multiple aliases that are deliberately not deprecated (e.g.
"shell.env" and "env"). See email thread "[PROPOSAL] Deprecate config
key aliases" to follow shortly.
If desired, we could support `.alias("my.toy.example.otherName")` on
the config key.
==========
Part 3
==========
We currently also support the `@SetFromFlag` annotation on fields
(let's focus here on entity, location, enricher and policy).
The value of the field is persisted, and is set on rebind. It behaves
like an attribute that can be set via a config key. Its use has been
discouraged (on entities, at least) for quite some time, but not
formally deprecated.
_Proposal_
For entities, we should formally deprecated this - warning in any
entity that uses it - and delete support for it in a couple of
releases time.
For locations/policies/enrichers, I think we should also deprecated
it. Unfortunately they don't support "attributes", but they do
support
reconfigurable config so that can be used instead.
==========
Part 4
==========
One can use @SetFromFlag on fields of other objects, which allows the
fields to be set when using the DSL `$brooklyn:object`.
I think this will become redundant with Alex's YOML. Let's not
discuss
such usage of @SetFromFlag in this email thread.
