brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Heneveld <alex.henev...@cloudsoftcorp.com>
Subject Re: [PROPOSAL] Deprecate config key aliases / SetFromFlag
Date Tue, 22 Nov 2016 22:31:05 GMT

Hi Aled -

There are a few more things that I think need to be considered here.  
Also, combining your proposals.

Firstly -- throughout Brooklyn we use the long-name syntax as an 
internal / formal name of a key to prevent ambiguity, and a short "flag 
name" to make it easy for a user to write.  This is sometimes useful 
where you set a config key at the root, using the formal name, so that 
it is inherited at a specific descendant.  If we don't need to rely on 
inheritance this argument goes away somewhat but I don't think we're 
there yet.

Secondly -- what problem are we trying to solve?  I think we should be 
dismissive of proposals that don't solve a problem. Yours does but it 
doesn't say what that problem is.  I think the problem is that having 
multiple ways to do the same thing can be confusing, especially if docs 
and examples are inconsistent.

I think a better solution that that problem is a clear *preferred* way 
-- a canonical form if you like -- for any blueprint, and the ability to 
output things in that format.

This means users looking at our docs and examples -- or anything that 
uses canonical forms -- will see a consistent style.  It eliminates 
some, if not all, of the confusion which is the problem.

It also gives us an easy way to update a blueprint where things are 
deprecated/changed.

I'd much prefer going that route than the proposal you suggest, and then 
deciding after that whether deprecating all aliases is the right thing.  
(Given the short/long distinction I'd prefer the idea that "all-but-one" 
alias might be deprecated in most cases. But I'm also unsure that with a 
canonical form and good tooling, aliases might actually be a good 
thing.  They are commonplace in more text interactive scripting -- which 
this approaches, as opposed to the method names in Aled's proposal.  
Think of CLI arguments and of course the text adventure games of our 
youth...

(As you know this is largely implemented in #363, at which point 
deprecated @SetFromFlag and moving to preferred aliases becomes simple, 
and optionally saying that all or any non-preferred alias is deprecated.)

--A

[1] https://github.com/apache/brooklyn-server/pull/363


On 22/11/2016 21:48, Aled Sage wrote:
> 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
View raw message