You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by Aled Sage <al...@gmail.com> on 2016/11/22 21:48:59 UTC

[PROPOSAL] Deprecate config key aliases

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.


Re: [PROPOSAL] Deprecate config key aliases / SetFromFlag

Posted by Sam Corbett <sa...@cloudsoftcorp.com>.
I would like to see aliases for configuration deprecated. I can't think 
of an instance in which we should support two names for one option. In 
my experience aliases lead to nothing but confusion for Brooklyn users. 
Why does the documentation say one thing when the blueprint I'm testing 
says another? Are they the same? Sometimes yes, sometimes no. Argh! 
Tooling might alleviate the problem but I think that right now many to 
most of our users will write blueprints in their text editor, a place 
such dynamic support would find it hard to reach.

A few emails up the chain Alex wrote about aliases to avoid ambiguity: 
"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." In 
the cases that a short name for a config key is inappropriate for use at 
the root the pattern in both the blueprints that I've written and that 
I've seen other people write has been effectively to rename them to 
avoid the ambiguity. For example, in an app with two clusters that use 
the "size" parameter you would write:

   app
     config:
       foo-cluster-size: 3
       bar-cluster-size: 5
     children:
       foo-cluster:
         size: $brooklyn:config("foo-cluster-size")
       bar-cluster:
         size: $brooklyn:config("bar-cluster-size")

This is just another way of aliasing things, but crucially the choice is 
the blueprint author's. As soon as a value gets into a Brooklyn entity I 
want one true form.

Sam


On 23/11/2016 11:26, Aled Sage wrote:
> Thanks Alex.
>
> For aliases for the _type_, agreed. We'll need to be careful how we 
> choose/register those aliases. That feels like a different discussion 
> from this one, which focuses on configKey names though.
>
> ---
> For "replace the ad hoc code for different types e.g. EntitySpec, 
> PolicySpec, ..."
>
> My understanding was that #363 would radically change our YAML parsing 
> (and later persistence). However, we would still generate the Java 
> objects like `EntitySpec` and still use the Entity Factory for 
> constructing them. I assumed we would still (at the java backend 
> level) have the ConfigKey class.
>
> I agree we can make other big improvements for getting rid of 
> differences between EntitySpec, PolicySpec etc, and differences 
> between how Entity and Policy handle config/attributes etc. Again, 
> that feels like a different topic.
>
> ---
> You said "for readability i'd say that a comfortable alias is nicer to 
> work with and should be the canonical form, as opposed to a long and 
> ugly identifier".
>
> I question whether we need the long and ugly identifier at all (except 
> as a deprecated name for backwards compatibility).
>
> Perhaps we also have a difference of opinion of how the alias should 
> behave. I think that, if it exists, it should be a genuine alias that 
> can be used anywhere that the other name(s) can be used. Do you think 
> we should have different rules for where the alias can be used and how 
> it behaves (e.g. never inherited in the runtime management hierarchy, 
> and/or never inherited by sub-types)?
>
> ---
> I'll respond on the separate email thread ("Deprecate @SetFromFlag") 
> about how best to move towards those usability improvements.
>
> (So this thread is just about whether we should have aliases - they 
> are separate questions).
>
> Aled
>
>
> On 23/11/2016 10:27, Alex Heneveld wrote:
>>
>> concrete example where aliases are essential in my view:  when 
>> entering the _type_.  how many of us switch to an IDE or grep to find 
>> the package.Class name then paste it in.  having `server` as an alias 
>> for the type makes it much easier to write yaml.
>>
>> my canonical form vision is that in the ui we can highlight wherever 
>> a non-canonical form is being used and give the option to switch.  
>> for readability i'd say that a comfortable alias is nicer to work 
>> with and should be the canonical form, as opposed to a long and ugly 
>> identifier.  so re your obj #1 we want some aliases:  it's not a 
>> dead-end code path.  discarding them altogether is going to cause 
>> other issues.  (that's not to say we can't clean up their usage, 
>> which of course we can.)
>>
>> btw #363 allows design time yaml to be converted to canonical form 
>> not just persisted state of deployed models (and yes, i think it 
>> should replace the ad hoc code for different types eg EntitySpec, 
>> PolicySpec, ....).
>>
>> the real issue in my view is that we have too many aliases and they 
>> are used inconsistently without good docs/help.  a canonical form and 
>> interactive help on keys will go a long way towards solving that.  
>> simply deprecating aliases without that is just going to be 
>> irritating.  similarly i'd like us to have a better tie in with 
>> source control and developer workflow before advocating a big change 
>> to blueprints (i think this will help with #2; ie have in-product 
>> warnings on deprecated usage and paths to upgrade, before we 
>> deprecate/drop aliases ... this is an issue either with your proposal 
>> or mine).
>>
>> so my strong preference is to focus on those usability items first, 
>> and *then* look at eliminating some aliases.  any other path is going 
>> to be even more disruptive for users!
>>
>> --a
>>
>>
>> On 23/11/2016 10:09, Aled Sage wrote:
>>> // The @SetFromFlag is an implementation detail - this proposal is 
>>> just to discuss whether we should have aliases.
>>>
>>> As Alex says, the main problem being solved is that having multiple 
>>> names makes things confusing (even if we were to fix other 
>>> inconsistencies so that those names could be used in the same way 
>>> anywhere).
>>>
>>> ---
>>> Alex suggests having a "clear preferred way -- a canonical form if 
>>> you like -- for any blueprint, and the ability to output things in 
>>> that format."
>>>
>>> Two things scare me about that:
>>>
>>> 1. It suggests that we go to the effort of supporting an alternative
>>>    name, but make sure we never use that alternate name in any of our
>>>    examples and thus try to make sure users don't use it. If so, why is
>>>    it there? Will it just lead to confusion when someone comes across a
>>>    blueprint that uses the short-form name, which is thus different
>>>    from all "official" examples?
>>> 2. For "output things in that format", their YAML blueprint is likely
>>>    in version control (or blog, or whatever). We are thus not changing
>>>    their blueprint. If they use a short-form name, then that will
>>>    continue to be in version control. If the blueprint is added to an
>>>    online catalog, it will continue to use that short-form name
>>>    (because we'd show in the catalog the exact blueprint from their git
>>>    repo).
>>>
>>> ---
>>> Alex says "It also gives us an easy way to update a blueprint where 
>>> things are deprecated/changed."
>>>
>>> I don't follow - are we talking about solving different problems, or 
>>> is your vision of PR #363 that we eventually replace the EntitySpec 
>>> and ConfigKey classes, and the `brooklyn.parameters` as well?
>>>
>>> Let's take a concrete use-cases. (let's not argue/discuss the 
>>> specific names, and instead focus on the use-cases):
>>>
>>> Someone writes a blueprint with `enricher.sourceSensor: cpuUsage`. 
>>> We deprecate "enricher.sourceSensor", preferring the name 
>>> "sourceSensor".
>>>
>>> The desired behaviour (IMO) is that:
>>>
>>> 1. We continue to support both names for X releases/months.
>>> 2. The blueprint author is warned about use of the deprecated name
>>>    (next time they validate, or next time they deploy).
>>> 3. The entity's type show the new name and deprecated name(s). This is
>>>    also included in auto-generated docs (similar to auto-generated
>>>    javadoc), and is available for Brooklyn's YAML composer to give
>>>    warnings (either while editing, or when the blueprint is submitted).
>>>
>>> I'm guessing what you mean by "easy way to update a blueprint" is 
>>> for the persisted state: to switch the name that is written to the 
>>> persisted state, so that it uses the new name. That is probably 
>>> good, but we should think carefully about implications for rolling 
>>> back to older Brooklyn versions.
>>>
>>> ---
>>> For comparing "long-name syntax" versus short "flag name" with CLIs...
>>>
>>> CLIs usually follow a very specific convention, e.g. `diff -w` and 
>>> `diff --ignore-all-space`; except for java which uses single "-" for 
>>> both short and long form - a very bad thing in my opinion :-)
>>>
>>> Some CLIs (like `br`) accept long and short forms (e.g. `br 
>>> application` or `br app`). This is ok because the context is never 
>>> ambiguous - you never pass "application" or "app" to a different 
>>> command, expecting different behaviour / ambiguity for it then 
>>> invoking `br`.
>>>
>>> ---
>>> If we conclude that aliases are sometimes a good idea, we should 
>>> agree when and how they should be used (e.g. is it primarily for 
>>> short-form; is it for multiple sensible names; is it for supporting 
>>> camel-case versus dot or underscore forms; etc).
>>>
>>> Unfortunately our aliases are massively over used in Brooklyn (in my 
>>> opinion), in an ad hoc manner. Most (if not all) should be deprecated.
>>>
>>> Aled
>>>
>>>
>>> On 22/11/2016 22:31, Alex Heneveld wrote:
>>>>
>>>> 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.
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>


Re: [PROPOSAL] Deprecate config key aliases / SetFromFlag

Posted by Aled Sage <al...@gmail.com>.
Thanks Alex.

For aliases for the _type_, agreed. We'll need to be careful how we 
choose/register those aliases. That feels like a different discussion 
from this one, which focuses on configKey names though.

---
For "replace the ad hoc code for different types e.g. EntitySpec, 
PolicySpec, ..."

My understanding was that #363 would radically change our YAML parsing 
(and later persistence). However, we would still generate the Java 
objects like `EntitySpec` and still use the Entity Factory for 
constructing them. I assumed we would still (at the java backend level) 
have the ConfigKey class.

I agree we can make other big improvements for getting rid of 
differences between EntitySpec, PolicySpec etc, and differences between 
how Entity and Policy handle config/attributes etc. Again, that feels 
like a different topic.

---
You said "for readability i'd say that a comfortable alias is nicer to 
work with and should be the canonical form, as opposed to a long and 
ugly identifier".

I question whether we need the long and ugly identifier at all (except 
as a deprecated name for backwards compatibility).

Perhaps we also have a difference of opinion of how the alias should 
behave. I think that, if it exists, it should be a genuine alias that 
can be used anywhere that the other name(s) can be used. Do you think we 
should have different rules for where the alias can be used and how it 
behaves (e.g. never inherited in the runtime management hierarchy, 
and/or never inherited by sub-types)?

---
I'll respond on the separate email thread ("Deprecate @SetFromFlag") 
about how best to move towards those usability improvements.

(So this thread is just about whether we should have aliases - they are 
separate questions).

Aled


On 23/11/2016 10:27, Alex Heneveld wrote:
>
> concrete example where aliases are essential in my view:  when 
> entering the _type_.  how many of us switch to an IDE or grep to find 
> the package.Class name then paste it in.  having `server` as an alias 
> for the type makes it much easier to write yaml.
>
> my canonical form vision is that in the ui we can highlight wherever a 
> non-canonical form is being used and give the option to switch.  for 
> readability i'd say that a comfortable alias is nicer to work with and 
> should be the canonical form, as opposed to a long and ugly 
> identifier.  so re your obj #1 we want some aliases:  it's not a 
> dead-end code path.  discarding them altogether is going to cause 
> other issues.  (that's not to say we can't clean up their usage, which 
> of course we can.)
>
> btw #363 allows design time yaml to be converted to canonical form not 
> just persisted state of deployed models (and yes, i think it should 
> replace the ad hoc code for different types eg EntitySpec, PolicySpec, 
> ....).
>
> the real issue in my view is that we have too many aliases and they 
> are used inconsistently without good docs/help.  a canonical form and 
> interactive help on keys will go a long way towards solving that.  
> simply deprecating aliases without that is just going to be 
> irritating.  similarly i'd like us to have a better tie in with source 
> control and developer workflow before advocating a big change to 
> blueprints (i think this will help with #2; ie have in-product 
> warnings on deprecated usage and paths to upgrade, before we 
> deprecate/drop aliases ... this is an issue either with your proposal 
> or mine).
>
> so my strong preference is to focus on those usability items first, 
> and *then* look at eliminating some aliases.  any other path is going 
> to be even more disruptive for users!
>
> --a
>
>
> On 23/11/2016 10:09, Aled Sage wrote:
>> // The @SetFromFlag is an implementation detail - this proposal is 
>> just to discuss whether we should have aliases.
>>
>> As Alex says, the main problem being solved is that having multiple 
>> names makes things confusing (even if we were to fix other 
>> inconsistencies so that those names could be used in the same way 
>> anywhere).
>>
>> ---
>> Alex suggests having a "clear preferred way -- a canonical form if 
>> you like -- for any blueprint, and the ability to output things in 
>> that format."
>>
>> Two things scare me about that:
>>
>> 1. It suggests that we go to the effort of supporting an alternative
>>    name, but make sure we never use that alternate name in any of our
>>    examples and thus try to make sure users don't use it. If so, why is
>>    it there? Will it just lead to confusion when someone comes across a
>>    blueprint that uses the short-form name, which is thus different
>>    from all "official" examples?
>> 2. For "output things in that format", their YAML blueprint is likely
>>    in version control (or blog, or whatever). We are thus not changing
>>    their blueprint. If they use a short-form name, then that will
>>    continue to be in version control. If the blueprint is added to an
>>    online catalog, it will continue to use that short-form name
>>    (because we'd show in the catalog the exact blueprint from their git
>>    repo).
>>
>> ---
>> Alex says "It also gives us an easy way to update a blueprint where 
>> things are deprecated/changed."
>>
>> I don't follow - are we talking about solving different problems, or 
>> is your vision of PR #363 that we eventually replace the EntitySpec 
>> and ConfigKey classes, and the `brooklyn.parameters` as well?
>>
>> Let's take a concrete use-cases. (let's not argue/discuss the 
>> specific names, and instead focus on the use-cases):
>>
>> Someone writes a blueprint with `enricher.sourceSensor: cpuUsage`. We 
>> deprecate "enricher.sourceSensor", preferring the name "sourceSensor".
>>
>> The desired behaviour (IMO) is that:
>>
>> 1. We continue to support both names for X releases/months.
>> 2. The blueprint author is warned about use of the deprecated name
>>    (next time they validate, or next time they deploy).
>> 3. The entity's type show the new name and deprecated name(s). This is
>>    also included in auto-generated docs (similar to auto-generated
>>    javadoc), and is available for Brooklyn's YAML composer to give
>>    warnings (either while editing, or when the blueprint is submitted).
>>
>> I'm guessing what you mean by "easy way to update a blueprint" is for 
>> the persisted state: to switch the name that is written to the 
>> persisted state, so that it uses the new name. That is probably good, 
>> but we should think carefully about implications for rolling back to 
>> older Brooklyn versions.
>>
>> ---
>> For comparing "long-name syntax" versus short "flag name" with CLIs...
>>
>> CLIs usually follow a very specific convention, e.g. `diff -w` and 
>> `diff --ignore-all-space`; except for java which uses single "-" for 
>> both short and long form - a very bad thing in my opinion :-)
>>
>> Some CLIs (like `br`) accept long and short forms (e.g. `br 
>> application` or `br app`). This is ok because the context is never 
>> ambiguous - you never pass "application" or "app" to a different 
>> command, expecting different behaviour / ambiguity for it then 
>> invoking `br`.
>>
>> ---
>> If we conclude that aliases are sometimes a good idea, we should 
>> agree when and how they should be used (e.g. is it primarily for 
>> short-form; is it for multiple sensible names; is it for supporting 
>> camel-case versus dot or underscore forms; etc).
>>
>> Unfortunately our aliases are massively over used in Brooklyn (in my 
>> opinion), in an ad hoc manner. Most (if not all) should be deprecated.
>>
>> Aled
>>
>>
>> On 22/11/2016 22:31, Alex Heneveld wrote:
>>>
>>> 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.
>>>>
>>>>
>>>
>>
>>
>


Re: [PROPOSAL] Deprecate config key aliases / SetFromFlag

Posted by Alex Heneveld <al...@cloudsoftcorp.com>.
concrete example where aliases are essential in my view:  when entering 
the _type_.  how many of us switch to an IDE or grep to find the 
package.Class name then paste it in.  having `server` as an alias for 
the type makes it much easier to write yaml.

my canonical form vision is that in the ui we can highlight wherever a 
non-canonical form is being used and give the option to switch.  for 
readability i'd say that a comfortable alias is nicer to work with and 
should be the canonical form, as opposed to a long and ugly identifier.  
so re your obj #1 we want some aliases:  it's not a dead-end code path.  
discarding them altogether is going to cause other issues.  (that's not 
to say we can't clean up their usage, which of course we can.)

btw #363 allows design time yaml to be converted to canonical form not 
just persisted state of deployed models (and yes, i think it should 
replace the ad hoc code for different types eg EntitySpec, PolicySpec, 
....).

the real issue in my view is that we have too many aliases and they are 
used inconsistently without good docs/help.  a canonical form and 
interactive help on keys will go a long way towards solving that.  
simply deprecating aliases without that is just going to be irritating.  
similarly i'd like us to have a better tie in with source control and 
developer workflow before advocating a big change to blueprints (i think 
this will help with #2; ie have in-product warnings on deprecated usage 
and paths to upgrade, before we deprecate/drop aliases ... this is an 
issue either with your proposal or mine).

so my strong preference is to focus on those usability items first, and 
*then* look at eliminating some aliases.  any other path is going to be 
even more disruptive for users!

--a


On 23/11/2016 10:09, Aled Sage wrote:
> // The @SetFromFlag is an implementation detail - this proposal is 
> just to discuss whether we should have aliases.
>
> As Alex says, the main problem being solved is that having multiple 
> names makes things confusing (even if we were to fix other 
> inconsistencies so that those names could be used in the same way 
> anywhere).
>
> ---
> Alex suggests having a "clear preferred way -- a canonical form if you 
> like -- for any blueprint, and the ability to output things in that 
> format."
>
> Two things scare me about that:
>
> 1. It suggests that we go to the effort of supporting an alternative
>    name, but make sure we never use that alternate name in any of our
>    examples and thus try to make sure users don't use it. If so, why is
>    it there? Will it just lead to confusion when someone comes across a
>    blueprint that uses the short-form name, which is thus different
>    from all "official" examples?
> 2. For "output things in that format", their YAML blueprint is likely
>    in version control (or blog, or whatever). We are thus not changing
>    their blueprint. If they use a short-form name, then that will
>    continue to be in version control. If the blueprint is added to an
>    online catalog, it will continue to use that short-form name
>    (because we'd show in the catalog the exact blueprint from their git
>    repo).
>
> ---
> Alex says "It also gives us an easy way to update a blueprint where 
> things are deprecated/changed."
>
> I don't follow - are we talking about solving different problems, or 
> is your vision of PR #363 that we eventually replace the EntitySpec 
> and ConfigKey classes, and the `brooklyn.parameters` as well?
>
> Let's take a concrete use-cases. (let's not argue/discuss the specific 
> names, and instead focus on the use-cases):
>
> Someone writes a blueprint with `enricher.sourceSensor: cpuUsage`. We 
> deprecate "enricher.sourceSensor", preferring the name "sourceSensor".
>
> The desired behaviour (IMO) is that:
>
> 1. We continue to support both names for X releases/months.
> 2. The blueprint author is warned about use of the deprecated name
>    (next time they validate, or next time they deploy).
> 3. The entity's type show the new name and deprecated name(s). This is
>    also included in auto-generated docs (similar to auto-generated
>    javadoc), and is available for Brooklyn's YAML composer to give
>    warnings (either while editing, or when the blueprint is submitted).
>
> I'm guessing what you mean by "easy way to update a blueprint" is for 
> the persisted state: to switch the name that is written to the 
> persisted state, so that it uses the new name. That is probably good, 
> but we should think carefully about implications for rolling back to 
> older Brooklyn versions.
>
> ---
> For comparing "long-name syntax" versus short "flag name" with CLIs...
>
> CLIs usually follow a very specific convention, e.g. `diff -w` and 
> `diff --ignore-all-space`; except for java which uses single "-" for 
> both short and long form - a very bad thing in my opinion :-)
>
> Some CLIs (like `br`) accept long and short forms (e.g. `br 
> application` or `br app`). This is ok because the context is never 
> ambiguous - you never pass "application" or "app" to a different 
> command, expecting different behaviour / ambiguity for it then 
> invoking `br`.
>
> ---
> If we conclude that aliases are sometimes a good idea, we should agree 
> when and how they should be used (e.g. is it primarily for short-form; 
> is it for multiple sensible names; is it for supporting camel-case 
> versus dot or underscore forms; etc).
>
> Unfortunately our aliases are massively over used in Brooklyn (in my 
> opinion), in an ad hoc manner. Most (if not all) should be deprecated.
>
> Aled
>
>
> On 22/11/2016 22:31, Alex Heneveld wrote:
>>
>> 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.
>>>
>>>
>>
>
>


Re: [PROPOSAL] Deprecate config key aliases / SetFromFlag

Posted by Svetoslav Neykov <sv...@cloudsoftcorp.com>.
I think that a single supported name for config keys is the right way to go. Agree with the proposal.
I haven't seen a convincing example in this thread of why config key aliases would be useful. For UI purposes we could have "keywords" that point you to a key you might be looking for, but upon selection you'd get the only supported name for that key. The description could include those keywords or they could live in a separate entry if it really need its own space. This way it's possible to do exploratory programming but in the end you get a canonical yaml. When looking at reference documentation you again see the same key name and use it, without being offered alternative list of names for that key. 

People's expectations are that there's one and only way to set a value in the blueprint. I've had to explain the difference between (for example) env & shell.env and that's surprising behaviour to users. They are used to reference documentation containing a single key that they can use.

Having deprecated aliases is a valid use case so +1. We are not at a point where we can warn users in the UI for using deprecated names, but even having the documentation spell them out as deprecated and pointing users to a link is very useful. I expect this functionality to get used infrequently. Mostly for typos or names which turn out to be ambiguous. 

Svet.


> On 5.12.2016 г., at 16:44, Andrew Kennedy <an...@cloudsoftcorp.com> wrote:
> 
> I think that there are always tradeoffs when you introduce flexibility, and
> I don't know that the flexibility of multiple names for config keys
> provides any specific benefit, but just adds confusion. But, we end up with
> the problem that we support it just now, so we cannot remove the capability
> without breaking existing blueprints.
> 
> So, I would like to see the short form config keys deprecated.
> 
> As to the path required to make this happen, I guess we need to have a
> mechanism that will warn blueprint authors that @SetFromFlag names are
> deprecated. We don't need to actually force them to stop using them, or
> handle a way of changing the short form to the canonical form *yet* but
> that seems like it will be the next step.
> 
> I think the idea of a canonical form of our YAML is a good idea, and if
> YOML will enable this, we should definitely go down that path. This raises
> the additional issue of *where* the config keys should be defined (even if
> they have the right names) - either at root or in brooklyn.config. I
> personally favour a canonical use of brooklyn.config here, and it would be
> awesome to have a tool like gofmt that could canonicalize any of our
> blueprints.
> 
> Once we have these capabilities we can move beyond just semantically
> deprecating the various old formats of YAML blueprints, and actually go and
> think about the process for removing support.
> 
> Finally, it seems like we might find it useful to version out blueprint
> YAML formats, and maybe these changes should happen in something we define
> as Brooklyn YAML 2.0? This would maybe accompany the YOML support?
> 
> Thanks,
> Andrew.
> 
> On Wed, 23 Nov 2016 at 10:09 Aled Sage <al...@gmail.com> wrote:
> 
> // The @SetFromFlag is an implementation detail - this proposal is just
> to discuss whether we should have aliases.
> 
> As Alex says, the main problem being solved is that having multiple
> names makes things confusing (even if we were to fix other
> inconsistencies so that those names could be used in the same way anywhere).
> 
> ---
> Alex suggests having a "clear preferred way -- a canonical form if you
> like -- for any blueprint, and the ability to output things in that format."
> 
> Two things scare me about that:
> 
> 1. It suggests that we go to the effort of supporting an alternative
>    name, but make sure we never use that alternate name in any of our
>    examples and thus try to make sure users don't use it. If so, why is
>    it there? Will it just lead to confusion when someone comes across a
>    blueprint that uses the short-form name, which is thus different
>    from all "official" examples?
> 2. For "output things in that format", their YAML blueprint is likely
>    in version control (or blog, or whatever). We are thus not changing
>    their blueprint. If they use a short-form name, then that will
>    continue to be in version control. If the blueprint is added to an
>    online catalog, it will continue to use that short-form name
>    (because we'd show in the catalog the exact blueprint from their git
>    repo).
> 
> ---
> Alex says "It also gives us an easy way to update a blueprint where
> things are deprecated/changed."
> 
> I don't follow - are we talking about solving different problems, or is
> your vision of PR #363 that we eventually replace the EntitySpec and
> ConfigKey classes, and the `brooklyn.parameters` as well?
> 
> Let's take a concrete use-cases. (let's not argue/discuss the specific
> names, and instead focus on the use-cases):
> 
> Someone writes a blueprint with `enricher.sourceSensor: cpuUsage`. We
> deprecate "enricher.sourceSensor", preferring the name "sourceSensor".
> 
> The desired behaviour (IMO) is that:
> 
> 1. We continue to support both names for X releases/months.
> 2. The blueprint author is warned about use of the deprecated name
>    (next time they validate, or next time they deploy).
> 3. The entity's type show the new name and deprecated name(s). This is
>    also included in auto-generated docs (similar to auto-generated
>    javadoc), and is available for Brooklyn's YAML composer to give
>    warnings (either while editing, or when the blueprint is submitted).
> 
> I'm guessing what you mean by "easy way to update a blueprint" is for
> the persisted state: to switch the name that is written to the persisted
> state, so that it uses the new name. That is probably good, but we
> should think carefully about implications for rolling back to older
> Brooklyn versions.
> 
> ---
> For comparing "long-name syntax" versus short "flag name" with CLIs...
> 
> CLIs usually follow a very specific convention, e.g. `diff -w` and `diff
> --ignore-all-space`; except for java which uses single "-" for both
> short and long form - a very bad thing in my opinion :-)
> 
> Some CLIs (like `br`) accept long and short forms (e.g. `br application`
> or `br app`). This is ok because the context is never ambiguous - you
> never pass "application" or "app" to a different command, expecting
> different behaviour / ambiguity for it then invoking `br`.
> 
> ---
> If we conclude that aliases are sometimes a good idea, we should agree
> when and how they should be used (e.g. is it primarily for short-form;
> is it for multiple sensible names; is it for supporting camel-case
> versus dot or underscore forms; etc).
> 
> Unfortunately our aliases are massively over used in Brooklyn (in my
> opinion), in an ad hoc manner. Most (if not all) should be deprecated.
> 
> Aled
> 
> 
> On 22/11/2016 22:31, Alex Heneveld wrote:
>> 
>> 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.
>>> 
>>> 
>> 
> 
> -- 
> 
> Andrew Kennedy ; Founder clocker.io project ; @grkvlt ; Cloudsoft


Re: [PROPOSAL] Deprecate config key aliases / SetFromFlag

Posted by Andrew Kennedy <an...@cloudsoftcorp.com>.
I think that there are always tradeoffs when you introduce flexibility, and
I don't know that the flexibility of multiple names for config keys
provides any specific benefit, but just adds confusion. But, we end up with
the problem that we support it just now, so we cannot remove the capability
without breaking existing blueprints.

So, I would like to see the short form config keys deprecated.

As to the path required to make this happen, I guess we need to have a
mechanism that will warn blueprint authors that @SetFromFlag names are
deprecated. We don't need to actually force them to stop using them, or
handle a way of changing the short form to the canonical form *yet* but
that seems like it will be the next step.

I think the idea of a canonical form of our YAML is a good idea, and if
YOML will enable this, we should definitely go down that path. This raises
the additional issue of *where* the config keys should be defined (even if
they have the right names) - either at root or in brooklyn.config. I
personally favour a canonical use of brooklyn.config here, and it would be
awesome to have a tool like gofmt that could canonicalize any of our
blueprints.

Once we have these capabilities we can move beyond just semantically
deprecating the various old formats of YAML blueprints, and actually go and
think about the process for removing support.

Finally, it seems like we might find it useful to version out blueprint
YAML formats, and maybe these changes should happen in something we define
as Brooklyn YAML 2.0? This would maybe accompany the YOML support?

Thanks,
Andrew.

On Wed, 23 Nov 2016 at 10:09 Aled Sage <al...@gmail.com> wrote:

// The @SetFromFlag is an implementation detail - this proposal is just
to discuss whether we should have aliases.

As Alex says, the main problem being solved is that having multiple
names makes things confusing (even if we were to fix other
inconsistencies so that those names could be used in the same way anywhere).

---
Alex suggests having a "clear preferred way -- a canonical form if you
like -- for any blueprint, and the ability to output things in that format."

Two things scare me about that:

 1. It suggests that we go to the effort of supporting an alternative
    name, but make sure we never use that alternate name in any of our
    examples and thus try to make sure users don't use it. If so, why is
    it there? Will it just lead to confusion when someone comes across a
    blueprint that uses the short-form name, which is thus different
    from all "official" examples?
 2. For "output things in that format", their YAML blueprint is likely
    in version control (or blog, or whatever). We are thus not changing
    their blueprint. If they use a short-form name, then that will
    continue to be in version control. If the blueprint is added to an
    online catalog, it will continue to use that short-form name
    (because we'd show in the catalog the exact blueprint from their git
    repo).

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

I don't follow - are we talking about solving different problems, or is
your vision of PR #363 that we eventually replace the EntitySpec and
ConfigKey classes, and the `brooklyn.parameters` as well?

Let's take a concrete use-cases. (let's not argue/discuss the specific
names, and instead focus on the use-cases):

Someone writes a blueprint with `enricher.sourceSensor: cpuUsage`. We
deprecate "enricher.sourceSensor", preferring the name "sourceSensor".

The desired behaviour (IMO) is that:

 1. We continue to support both names for X releases/months.
 2. The blueprint author is warned about use of the deprecated name
    (next time they validate, or next time they deploy).
 3. The entity's type show the new name and deprecated name(s). This is
    also included in auto-generated docs (similar to auto-generated
    javadoc), and is available for Brooklyn's YAML composer to give
    warnings (either while editing, or when the blueprint is submitted).

I'm guessing what you mean by "easy way to update a blueprint" is for
the persisted state: to switch the name that is written to the persisted
state, so that it uses the new name. That is probably good, but we
should think carefully about implications for rolling back to older
Brooklyn versions.

---
For comparing "long-name syntax" versus short "flag name" with CLIs...

CLIs usually follow a very specific convention, e.g. `diff -w` and `diff
--ignore-all-space`; except for java which uses single "-" for both
short and long form - a very bad thing in my opinion :-)

Some CLIs (like `br`) accept long and short forms (e.g. `br application`
or `br app`). This is ok because the context is never ambiguous - you
never pass "application" or "app" to a different command, expecting
different behaviour / ambiguity for it then invoking `br`.

---
If we conclude that aliases are sometimes a good idea, we should agree
when and how they should be used (e.g. is it primarily for short-form;
is it for multiple sensible names; is it for supporting camel-case
versus dot or underscore forms; etc).

Unfortunately our aliases are massively over used in Brooklyn (in my
opinion), in an ad hoc manner. Most (if not all) should be deprecated.

Aled


On 22/11/2016 22:31, Alex Heneveld wrote:
>
> 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.
>>
>>
>

-- 

Andrew Kennedy ; Founder clocker.io project ; @grkvlt ; Cloudsoft

Re: [PROPOSAL] Deprecate config key aliases / SetFromFlag

Posted by Aled Sage <al...@gmail.com>.
// The @SetFromFlag is an implementation detail - this proposal is just 
to discuss whether we should have aliases.

As Alex says, the main problem being solved is that having multiple 
names makes things confusing (even if we were to fix other 
inconsistencies so that those names could be used in the same way anywhere).

---
Alex suggests having a "clear preferred way -- a canonical form if you 
like -- for any blueprint, and the ability to output things in that format."

Two things scare me about that:

 1. It suggests that we go to the effort of supporting an alternative
    name, but make sure we never use that alternate name in any of our
    examples and thus try to make sure users don't use it. If so, why is
    it there? Will it just lead to confusion when someone comes across a
    blueprint that uses the short-form name, which is thus different
    from all "official" examples?
 2. For "output things in that format", their YAML blueprint is likely
    in version control (or blog, or whatever). We are thus not changing
    their blueprint. If they use a short-form name, then that will
    continue to be in version control. If the blueprint is added to an
    online catalog, it will continue to use that short-form name
    (because we'd show in the catalog the exact blueprint from their git
    repo).

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

I don't follow - are we talking about solving different problems, or is 
your vision of PR #363 that we eventually replace the EntitySpec and 
ConfigKey classes, and the `brooklyn.parameters` as well?

Let's take a concrete use-cases. (let's not argue/discuss the specific 
names, and instead focus on the use-cases):

Someone writes a blueprint with `enricher.sourceSensor: cpuUsage`. We 
deprecate "enricher.sourceSensor", preferring the name "sourceSensor".

The desired behaviour (IMO) is that:

 1. We continue to support both names for X releases/months.
 2. The blueprint author is warned about use of the deprecated name
    (next time they validate, or next time they deploy).
 3. The entity's type show the new name and deprecated name(s). This is
    also included in auto-generated docs (similar to auto-generated
    javadoc), and is available for Brooklyn's YAML composer to give
    warnings (either while editing, or when the blueprint is submitted).

I'm guessing what you mean by "easy way to update a blueprint" is for 
the persisted state: to switch the name that is written to the persisted 
state, so that it uses the new name. That is probably good, but we 
should think carefully about implications for rolling back to older 
Brooklyn versions.

---
For comparing "long-name syntax" versus short "flag name" with CLIs...

CLIs usually follow a very specific convention, e.g. `diff -w` and `diff 
--ignore-all-space`; except for java which uses single "-" for both 
short and long form - a very bad thing in my opinion :-)

Some CLIs (like `br`) accept long and short forms (e.g. `br application` 
or `br app`). This is ok because the context is never ambiguous - you 
never pass "application" or "app" to a different command, expecting 
different behaviour / ambiguity for it then invoking `br`.

---
If we conclude that aliases are sometimes a good idea, we should agree 
when and how they should be used (e.g. is it primarily for short-form; 
is it for multiple sensible names; is it for supporting camel-case 
versus dot or underscore forms; etc).

Unfortunately our aliases are massively over used in Brooklyn (in my 
opinion), in an ad hoc manner. Most (if not all) should be deprecated.

Aled


On 22/11/2016 22:31, Alex Heneveld wrote:
>
> 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.
>>
>>
>


Re: [PROPOSAL] Deprecate config key aliases / SetFromFlag

Posted by Alex Heneveld <al...@cloudsoftcorp.com>.
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.
>
>