brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Aled Sage <aled.s...@gmail.com>
Subject [PROPOSAL] Deprecate config key aliases
Date Tue, 22 Nov 2016 21:48:59 GMT
Hi all,

TL;DR: aliases for config keys should be deprecated. Each config key 
should have only one proper name, with other names deprecated.

We should change this *after* releasing 0.10.0, to decrease risk.

(This was mentioned in the email thread "[PROPOSAL] Deprecate 
@SetFromFlag").

_*Current Situation*_
When defining a config keys in Java, one can add an annotation like:

    @SetFromFlag("version")
    ConfigKey<String> SUGGESTED_VERSION =
    newStringConfigKey("install.version", "Suggested version");

This alternative name specified in @SetFromFlag is respected in some 
situations, but not others.

_*Requirements*_
The desire to support multiple names can be split into three different 
use-cases:

 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.
    It is covered in the email thread "[PROPOSAL] Deprecate @SetFromFlag".
 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: Don't Use Aliases For Config Keys*_
I propose that we deprecate support for use-case (2) above: use of aliases.

The use of aliases leads to confusion about what the different names 
mean. When someone is looking at examples, it's unclear whether they 
mean the same thing, or if one is valid but the other is not. There is a 
scary amount of folk lore about config key names already!

Example blueprints have a tendency to proliferate: a blueprint written 
within a company adopting Brooklyn is often used as the basis for other 
blueprints. If we support an alias without a very obvious deprecation 
warning in the YAML composer, then use of that alias will spread.

---
Note that this is a separate discussion from whether our existing names 
are right! There are probably a lot of names we should deprecate and 
improve.

_*Proposal: Guidelines for "deprecated"*_
For use-case (1) above, i.e. deprecated names, we should treat that in a 
similar way to Java deprecated methods.

We should *not* add a deprecated name just because we think it's a nice 
alternative name. We should only add deprecated names when it is an 
undesirable name that we need to support for backwards compatibility.

For example, if someone submitted a pull request with three methods that 
all did the same thing, then I'd reject that PR - e.g. sort(collection), 
arrange(collection) and order(collection).

_*Proposal: Hints for Names*_
There is a compelling argument for providing hints for incorrect names, 
particularly when using an online YAML composer or when validating a 
YAML blueprint.

For example, if someone uses "environment.variables" but the real name 
is "env", then a validation warning can be shown with an error message 
proposing the correct name.

This could be achieved by providing "close names". If the name matches 
another config key, then that would be used. Otherwise, if the name 
matches a "close name" of a config key, then it would show the 
validation warning. Note that it is a warning rather than an error 
because of the rules for config inheritance: it could be that the config 
key will be inherited by children that will understand the given name.

We could have a "strict" mode that treated such warnings as errors 
(sounds like a topic for a different email thread!).

We could do some similar automatic checks for close matches, e.g. to 
warn if "installCommand" is used instead of "install.command".

To me, it feels like "hints" is stage two - i.e. lower priority than 
agreeing each config key should have a single definitive name, and 
deprecating the other names.


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message