[discuss] PropertyDescriptor name and displayName attributes

classic Classic list List threaded Threaded
22 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[discuss] PropertyDescriptor name and displayName attributes

Andy LoPresto-2
Hi all,

As a result of some conversations and some varying feedback on PRs, I’d like to discuss with the community an issue I see with PropertyDescriptor name and displayName attributes. I’ll describe the scenarios that cause issues and my proposed solution, and then solicit responses and other perspectives. 

A PropertyDescriptor has various attributes [1]. When the property is configured, a “name” is provided to uniquely identify the property. This name is both displayed on the UI in a property configuration dialog, and used in the REST API to retrieve or set values. When the flow is persisted to the flow.xml.gz file, the name identifies the value during serialization. 

There are multiple scenarios where the name value could be changed:

* There is a typo in the name
* The name is unclear or could be improved to more accurately reflect the purpose of the property (I believe we have had a couple instances with “batch” meaning when integrating with other projects)
* Internationalization and localization

When an existing PropertyDescriptor name is changed for any of these reasons, it breaks backward compatibility because a flow.xml.gz file which defines a value for the property name will no longer have that value retrieved [2]. In this case, name is serving a dual role for both UI display and object resolution within the persisted state. 

To address this, the displayName attribute was added to PropertyDescriptor [3]. This attribute allows a “human readable” name to be provided for UI purposes and modified at will without modifying the static name value. However, many developers are unaware of this attribute [4], and provide only the name attribute when contributing a new Processor. 

My proposal is to do the following:

* Improve the documentation to increase awareness of the displayName attribute and the benefit it provides
* Consciously encourage contributors to provide both name and displayName attributes on new processors and add displayName to existing processors during PR reviews
* Add code to warn (without blocking) on processors missing displayName attributes

I appreciate that providing both attributes may seem duplicative in the scenario where both are similar English phrases, which is the default today. However, as our community grows and we are seeing increased internationalization and localization efforts, I believe this will pay dividends. I also think being proactive by providing both attributes will increase developer awareness and avoid a scenario where a user changes the existing name attribute rather than add a displayName attribute. I feel the steps I outline above will get the maximum return with minimal coding effort and no changes to backward compatibility. 

I welcome the community’s feedback on this. 



Andy LoPresto
PGP Fingerprint: 70EC B3E5 98A6 5A3F D3C4  BACE 3C6E F65B 2F7D EF69


signature.asc (859 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [discuss] PropertyDescriptor name and displayName attributes

Oleg Zhurakousky
I am definitely +1 on this.
The only question I have is related to "Add code to warn (without blocking) on processors missing displayName attributes”. Did you mean in he code itself where some validator in the abstract class would flag it with a WARN or some build plugin?

Cheers
Oleg

On May 3, 2016, at 2:09 PM, Andy LoPresto <[hidden email]<mailto:[hidden email]>> wrote:

Hi all,

As a result of some conversations and some varying feedback on PRs, I’d like to discuss with the community an issue I see with PropertyDescriptor name and displayName attributes. I’ll describe the scenarios that cause issues and my proposed solution, and then solicit responses and other perspectives.

A PropertyDescriptor has various attributes [1]. When the property is configured, a “name” is provided to uniquely identify the property. This name is both displayed on the UI in a property configuration dialog, and used in the REST API to retrieve or set values. When the flow is persisted to the flow.xml.gz file, the name identifies the value during serialization.

There are multiple scenarios where the name value could be changed:

* There is a typo in the name
* The name is unclear or could be improved to more accurately reflect the purpose of the property (I believe we have had a couple instances with “batch” meaning when integrating with other projects)
* Internationalization and localization

When an existing PropertyDescriptor name is changed for any of these reasons, it breaks backward compatibility because a flow.xml.gz file which defines a value for the property name will no longer have that value retrieved [2]. In this case, name is serving a dual role for both UI display and object resolution within the persisted state.

To address this, the displayName attribute was added to PropertyDescriptor [3]. This attribute allows a “human readable” name to be provided for UI purposes and modified at will without modifying the static name value. However, many developers are unaware of this attribute [4], and provide only the name attribute when contributing a new Processor.

My proposal is to do the following:

* Improve the documentation to increase awareness of the displayName attribute and the benefit it provides
* Consciously encourage contributors to provide both name and displayName attributes on new processors and add displayName to existing processors during PR reviews
* Add code to warn (without blocking) on processors missing displayName attributes

I appreciate that providing both attributes may seem duplicative in the scenario where both are similar English phrases, which is the default today. However, as our community grows and we are seeing increased internationalization and localization efforts, I believe this will pay dividends. I also think being proactive by providing both attributes will increase developer awareness and avoid a scenario where a user changes the existing name attribute rather than add a displayName attribute. I feel the steps I outline above will get the maximum return with minimal coding effort and no changes to backward compatibility.

I welcome the community’s feedback on this.


[1] https://nifi.apache.org/docs/nifi-docs/html/developer-guide.html#documenting-properties
[2] https://issues.apache.org/jira/browse/NIFI-1795
[3] https://github.com/apache/nifi/blob/master/nifi-api/src/main/java/org/apache/nifi/components/PropertyDescriptor.java#L254
[4] https://issues.apache.org/jira/browse/NIFI-1828

Andy LoPresto
[hidden email]<mailto:[hidden email]>
[hidden email]<mailto:[hidden email]>
PGP Fingerprint: 70EC B3E5 98A6 5A3F D3C4  BACE 3C6E F65B 2F7D EF69


Reply | Threaded
Open this post in threaded view
|

Re: [discuss] PropertyDescriptor name and displayName attributes

Mark Payne
I'm a +0 here. However, the third bullet point that you listed, and the one Oleg is asking for clarification on here:

* Add code to warn (without blocking) on processors missing displayName attributes

I would recommend that we address this within the scope of NIFI-34 [1]. I.e, we don't want to fill the logs with WARN
messages, and we shouldn't throw an Exception if the Display Name is left out. But we should have the testing framework
fail the unit test, marking this as a bad practice.

This would be especially important if we create a Property Descriptor to use as a 'template' by using the fromPropertyDescriptor()
method of the builder. With NIFI-34, we would have the ability in a Unit Test to explicitly indicate that the check should not be
performed.

Thanks
-Mark


[1] https://issues.apache.org/jira/browse/NIFI-34 <https://issues.apache.org/jira/browse/NIFI-34>



> On May 3, 2016, at 2:15 PM, Oleg Zhurakousky <[hidden email]> wrote:
>
> I am definitely +1 on this.
> The only question I have is related to "Add code to warn (without blocking) on processors missing displayName attributes”. Did you mean in he code itself where some validator in the abstract class would flag it with a WARN or some build plugin?
>
> Cheers
> Oleg
>
> On May 3, 2016, at 2:09 PM, Andy LoPresto <[hidden email] <mailto:[hidden email]><mailto:[hidden email] <mailto:[hidden email]>>> wrote:
>
> Hi all,
>
> As a result of some conversations and some varying feedback on PRs, I’d like to discuss with the community an issue I see with PropertyDescriptor name and displayName attributes. I’ll describe the scenarios that cause issues and my proposed solution, and then solicit responses and other perspectives.
>
> A PropertyDescriptor has various attributes [1]. When the property is configured, a “name” is provided to uniquely identify the property. This name is both displayed on the UI in a property configuration dialog, and used in the REST API to retrieve or set values. When the flow is persisted to the flow.xml.gz file, the name identifies the value during serialization.
>
> There are multiple scenarios where the name value could be changed:
>
> * There is a typo in the name
> * The name is unclear or could be improved to more accurately reflect the purpose of the property (I believe we have had a couple instances with “batch” meaning when integrating with other projects)
> * Internationalization and localization
>
> When an existing PropertyDescriptor name is changed for any of these reasons, it breaks backward compatibility because a flow.xml.gz file which defines a value for the property name will no longer have that value retrieved [2]. In this case, name is serving a dual role for both UI display and object resolution within the persisted state.
>
> To address this, the displayName attribute was added to PropertyDescriptor [3]. This attribute allows a “human readable” name to be provided for UI purposes and modified at will without modifying the static name value. However, many developers are unaware of this attribute [4], and provide only the name attribute when contributing a new Processor.
>
> My proposal is to do the following:
>
> * Improve the documentation to increase awareness of the displayName attribute and the benefit it provides
> * Consciously encourage contributors to provide both name and displayName attributes on new processors and add displayName to existing processors during PR reviews
> * Add code to warn (without blocking) on processors missing displayName attributes
>
> I appreciate that providing both attributes may seem duplicative in the scenario where both are similar English phrases, which is the default today. However, as our community grows and we are seeing increased internationalization and localization efforts, I believe this will pay dividends. I also think being proactive by providing both attributes will increase developer awareness and avoid a scenario where a user changes the existing name attribute rather than add a displayName attribute. I feel the steps I outline above will get the maximum return with minimal coding effort and no changes to backward compatibility.
>
> I welcome the community’s feedback on this.
>
>
> [1] https://nifi.apache.org/docs/nifi-docs/html/developer-guide.html#documenting-properties
> [2] https://issues.apache.org/jira/browse/NIFI-1795
> [3] https://github.com/apache/nifi/blob/master/nifi-api/src/main/java/org/apache/nifi/components/PropertyDescriptor.java#L254
> [4] https://issues.apache.org/jira/browse/NIFI-1828
>
> Andy LoPresto
> [hidden email] <mailto:[hidden email]><mailto:[hidden email] <mailto:[hidden email]>>
> [hidden email] <mailto:[hidden email]><mailto:[hidden email] <mailto:[hidden email]>>
> PGP Fingerprint: 70EC B3E5 98A6 5A3F D3C4  BACE 3C6E F65B 2F7D EF69

Reply | Threaded
Open this post in threaded view
|

Re: [discuss] PropertyDescriptor name and displayName attributes

Andy LoPresto-2
Mark,

I am fine including it in the best practices validator linked in your message, but it doesn’t look like there has been any work on that ticket since January 2016. 

As a quick solution (and to answer Oleg’s question), I think we could add a simple Checkstyle regex rule [1] to detect the presence of displayName attribute in PropertyDescriptor declarations. We should work to clean up the deprecation warnings that currently print during a build in order to make any messages easier to read and more impactful.  



Andy LoPresto
PGP Fingerprint: 70EC B3E5 98A6 5A3F D3C4  BACE 3C6E F65B 2F7D EF69

On May 3, 2016, at 11:51 AM, Mark Payne <[hidden email]> wrote:

I'm a +0 here. However, the third bullet point that you listed, and the one Oleg is asking for clarification on here:

* Add code to warn (without blocking) on processors missing displayName attributes

I would recommend that we address this within the scope of NIFI-34 [1]. I.e, we don't want to fill the logs with WARN
messages, and we shouldn't throw an Exception if the Display Name is left out. But we should have the testing framework
fail the unit test, marking this as a bad practice.

This would be especially important if we create a Property Descriptor to use as a 'template' by using the fromPropertyDescriptor()
method of the builder. With NIFI-34, we would have the ability in a Unit Test to explicitly indicate that the check should not be
performed.

Thanks
-Mark


[1] https://issues.apache.org/jira/browse/NIFI-34 <https://issues.apache.org/jira/browse/NIFI-34>



On May 3, 2016, at 2:15 PM, Oleg Zhurakousky <[hidden email]> wrote:

I am definitely +1 on this.
The only question I have is related to "Add code to warn (without blocking) on processors missing displayName attributes”. Did you mean in he code itself where some validator in the abstract class would flag it with a WARN or some build plugin?

Cheers
Oleg

On May 3, 2016, at 2:09 PM, Andy LoPresto <[hidden email] <[hidden email]><[hidden email] <[hidden email]>>> wrote:

Hi all,

As a result of some conversations and some varying feedback on PRs, I’d like to discuss with the community an issue I see with PropertyDescriptor name and displayName attributes. I’ll describe the scenarios that cause issues and my proposed solution, and then solicit responses and other perspectives.

A PropertyDescriptor has various attributes [1]. When the property is configured, a “name” is provided to uniquely identify the property. This name is both displayed on the UI in a property configuration dialog, and used in the REST API to retrieve or set values. When the flow is persisted to the flow.xml.gz file, the name identifies the value during serialization.

There are multiple scenarios where the name value could be changed:

* There is a typo in the name
* The name is unclear or could be improved to more accurately reflect the purpose of the property (I believe we have had a couple instances with “batch” meaning when integrating with other projects)
* Internationalization and localization

When an existing PropertyDescriptor name is changed for any of these reasons, it breaks backward compatibility because a flow.xml.gz file which defines a value for the property name will no longer have that value retrieved [2]. In this case, name is serving a dual role for both UI display and object resolution within the persisted state.

To address this, the displayName attribute was added to PropertyDescriptor [3]. This attribute allows a “human readable” name to be provided for UI purposes and modified at will without modifying the static name value. However, many developers are unaware of this attribute [4], and provide only the name attribute when contributing a new Processor.

My proposal is to do the following:

* Improve the documentation to increase awareness of the displayName attribute and the benefit it provides
* Consciously encourage contributors to provide both name and displayName attributes on new processors and add displayName to existing processors during PR reviews
* Add code to warn (without blocking) on processors missing displayName attributes

I appreciate that providing both attributes may seem duplicative in the scenario where both are similar English phrases, which is the default today. However, as our community grows and we are seeing increased internationalization and localization efforts, I believe this will pay dividends. I also think being proactive by providing both attributes will increase developer awareness and avoid a scenario where a user changes the existing name attribute rather than add a displayName attribute. I feel the steps I outline above will get the maximum return with minimal coding effort and no changes to backward compatibility.

I welcome the community’s feedback on this.


[1] https://nifi.apache.org/docs/nifi-docs/html/developer-guide.html#documenting-properties
[2] https://issues.apache.org/jira/browse/NIFI-1795
[3] https://github.com/apache/nifi/blob/master/nifi-api/src/main/java/org/apache/nifi/components/PropertyDescriptor.java#L254
[4] https://issues.apache.org/jira/browse/NIFI-1828

Andy LoPresto
[hidden email] <[hidden email]><[hidden email] <[hidden email]>>
[hidden email] <[hidden email]><[hidden email] <[hidden email]>>
PGP Fingerprint: 70EC B3E5 98A6 5A3F D3C4  BACE 3C6E F65B 2F7D EF69



signature.asc (859 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [discuss] PropertyDescriptor name and displayName attributes

Adam Lamar
In reply to this post by Andy LoPresto-2
On Tue, May 3, 2016 at 12:09 PM, Andy LoPresto <[hidden email]> wrote:

> As a result of some conversations and some varying feedback on PRs, I’d like
> to discuss with the community an issue I see with PropertyDescriptor name
> and displayName attributes. I’ll describe the scenarios that cause issues
> and my proposed solution, and then solicit responses and other perspectives.

Andy,

I'd be +1 on this as well. I think the power of this approach will
become more clear over time, and some of the automation will make it
possible to more widely enforce.

What do you think about a mixed mode where config reading code can
fetch the property value with either the name or display name as the
key, defaulting to the name if it is present? A sample read of
flow.xml.gz might look like this:

* Processor asks for value of MY_CONFIG_PROPERTY
* Configuration code looks for key "my-config-property", returns if present
* Configuration code looks for key "My Config Property", returns if present
* On finding no valid key, configuration returns blank/default/null
value (whatever is done today)

On configuration write, the new attributes could be written as the
normalized name (instead of display name), to allow processors that
have made the switch to start using the normalized name field and
start taking advantage of the new features around it (e.g.
internationalization). Perhaps a disadvantage to this approach is that
it auto-upgrades an existing flow.xml.gz, making it difficult to
downgrade from (for example) 0.7 back to 0.6.

A strategy like this (or something similar) might help speed adoption
by making the process a bit smoother for existing flow.xml.gz files
and easier to upgrade processors incrementally. At 1.0, display names
as configuration keys could be dropped, and the upgrade path for users
on old 0.x releases would be to upgrade to the latest 0.x before
making the jump to 1.0.

Cheers,
Adam
Reply | Threaded
Open this post in threaded view
|

Re: [discuss] PropertyDescriptor name and displayName attributes

Ricky Saltzer
An idea I had is to modify the API and use a setter that would require both
the name and the display name. That way and you deprecate the other two
over time.

Example:
setName(String name, String DisplayName)

This would require both values to be filled out, and since it's overloaded
you could introduce non intrusively.

I understand this might not fit with the Java builder pattern best practice.

On Tue, May 3, 2016 at 12:09 PM, Andy LoPresto <[hidden email]> wrote:

> As a result of some conversations and some varying feedback on PRs, I’d
like
> to discuss with the community an issue I see with PropertyDescriptor name
> and displayName attributes. I’ll describe the scenarios that cause issues
> and my proposed solution, and then solicit responses and other
perspectives.

Andy,

I'd be +1 on this as well. I think the power of this approach will
become more clear over time, and some of the automation will make it
possible to more widely enforce.

What do you think about a mixed mode where config reading code can
fetch the property value with either the name or display name as the
key, defaulting to the name if it is present? A sample read of
flow.xml.gz might look like this:

* Processor asks for value of MY_CONFIG_PROPERTY
* Configuration code looks for key "my-config-property", returns if present
* Configuration code looks for key "My Config Property", returns if present
* On finding no valid key, configuration returns blank/default/null
value (whatever is done today)

On configuration write, the new attributes could be written as the
normalized name (instead of display name), to allow processors that
have made the switch to start using the normalized name field and
start taking advantage of the new features around it (e.g.
internationalization). Perhaps a disadvantage to this approach is that
it auto-upgrades an existing flow.xml.gz, making it difficult to
downgrade from (for example) 0.7 back to 0.6.

A strategy like this (or something similar) might help speed adoption
by making the process a bit smoother for existing flow.xml.gz files
and easier to upgrade processors incrementally. At 1.0, display names
as configuration keys could be dropped, and the upgrade path for users
on old 0.x releases would be to upgrade to the latest 0.x before
making the jump to 1.0.

Cheers,
Adam
Reply | Threaded
Open this post in threaded view
|

Re: [discuss] PropertyDescriptor name and displayName attributes

Brandon DeVries
In reply to this post by Adam Lamar
+1.  I think being able to move the displayName out of code an into config
/ ResourceBundle will make it much easier to support i18n[1].  If no config
is provided, the name is the default... otherwise, the name displayed is
determined by the locale.  Updating the mock framework to complain about
the absence of a ResourceBundle will encourage adoption, and we'll
gradually work our way to not being English only.

Brandon

[1] https://docs.oracle.com/javase/tutorial/i18n/intro/quick.html

On Thu, May 5, 2016 at 11:46 PM Adam Lamar <[hidden email]> wrote:

> On Tue, May 3, 2016 at 12:09 PM, Andy LoPresto <[hidden email]>
> wrote:
>
> > As a result of some conversations and some varying feedback on PRs, I’d
> like
> > to discuss with the community an issue I see with PropertyDescriptor name
> > and displayName attributes. I’ll describe the scenarios that cause issues
> > and my proposed solution, and then solicit responses and other
> perspectives.
>
> Andy,
>
> I'd be +1 on this as well. I think the power of this approach will
> become more clear over time, and some of the automation will make it
> possible to more widely enforce.
>
> What do you think about a mixed mode where config reading code can
> fetch the property value with either the name or display name as the
> key, defaulting to the name if it is present? A sample read of
> flow.xml.gz might look like this:
>
> * Processor asks for value of MY_CONFIG_PROPERTY
> * Configuration code looks for key "my-config-property", returns if present
> * Configuration code looks for key "My Config Property", returns if present
> * On finding no valid key, configuration returns blank/default/null
> value (whatever is done today)
>
> On configuration write, the new attributes could be written as the
> normalized name (instead of display name), to allow processors that
> have made the switch to start using the normalized name field and
> start taking advantage of the new features around it (e.g.
> internationalization). Perhaps a disadvantage to this approach is that
> it auto-upgrades an existing flow.xml.gz, making it difficult to
> downgrade from (for example) 0.7 back to 0.6.
>
> A strategy like this (or something similar) might help speed adoption
> by making the process a bit smoother for existing flow.xml.gz files
> and easier to upgrade processors incrementally. At 1.0, display names
> as configuration keys could be dropped, and the upgrade path for users
> on old 0.x releases would be to upgrade to the latest 0.x before
> making the jump to 1.0.
>
> Cheers,
> Adam
>
Reply | Threaded
Open this post in threaded view
|

Re: [discuss] PropertyDescriptor name and displayName attributes

Bryan Bende
I might just be resistant to change, but I am still on the fence a little
bit...

In the past the idea has always been you start out with name, and if you
later need to change what is displayed in the UI, then you add displayName
after the fact.

It sounds like the issue is that a lot of people don't know about
displayName, so I am totally in favor of increasing awareness through
documentation,
but I'm struggling with telling people that they should set displayName as
the default behavior.

For code that is contributed to NiFi, I would expect the PMC/committer
doing the review/merging to notice if an existing property name was being
changed and point that out in the review.
If it was someone else's custom NAR, or even made it through the review, I
think what happens is that the flow starts up and the processor/component
becomes invalid because it now has an unknown property.
This is the same behavior when we remove a property from an existing
processor and someone upgrades, and we deemed this acceptable behavior for
that scenario.

The internationalization aspect could possibly sell me more, but I think I
would need someone to explain how internationalization would be
implemented, and how setting the displayName helps.
What Brandon described makes sense to me because it is something outside
the Java code, which is different then saying all PropertyDescriptor
instances need name and displayName.

-Bryan


On Fri, May 6, 2016 at 8:44 AM, Brandon DeVries <[hidden email]> wrote:

> +1.  I think being able to move the displayName out of code an into config
> / ResourceBundle will make it much easier to support i18n[1].  If no config
> is provided, the name is the default... otherwise, the name displayed is
> determined by the locale.  Updating the mock framework to complain about
> the absence of a ResourceBundle will encourage adoption, and we'll
> gradually work our way to not being English only.
>
> Brandon
>
> [1] https://docs.oracle.com/javase/tutorial/i18n/intro/quick.html
>
> On Thu, May 5, 2016 at 11:46 PM Adam Lamar <[hidden email]> wrote:
>
> > On Tue, May 3, 2016 at 12:09 PM, Andy LoPresto <[hidden email]>
> > wrote:
> >
> > > As a result of some conversations and some varying feedback on PRs, I’d
> > like
> > > to discuss with the community an issue I see with PropertyDescriptor
> name
> > > and displayName attributes. I’ll describe the scenarios that cause
> issues
> > > and my proposed solution, and then solicit responses and other
> > perspectives.
> >
> > Andy,
> >
> > I'd be +1 on this as well. I think the power of this approach will
> > become more clear over time, and some of the automation will make it
> > possible to more widely enforce.
> >
> > What do you think about a mixed mode where config reading code can
> > fetch the property value with either the name or display name as the
> > key, defaulting to the name if it is present? A sample read of
> > flow.xml.gz might look like this:
> >
> > * Processor asks for value of MY_CONFIG_PROPERTY
> > * Configuration code looks for key "my-config-property", returns if
> present
> > * Configuration code looks for key "My Config Property", returns if
> present
> > * On finding no valid key, configuration returns blank/default/null
> > value (whatever is done today)
> >
> > On configuration write, the new attributes could be written as the
> > normalized name (instead of display name), to allow processors that
> > have made the switch to start using the normalized name field and
> > start taking advantage of the new features around it (e.g.
> > internationalization). Perhaps a disadvantage to this approach is that
> > it auto-upgrades an existing flow.xml.gz, making it difficult to
> > downgrade from (for example) 0.7 back to 0.6.
> >
> > A strategy like this (or something similar) might help speed adoption
> > by making the process a bit smoother for existing flow.xml.gz files
> > and easier to upgrade processors incrementally. At 1.0, display names
> > as configuration keys could be dropped, and the upgrade path for users
> > on old 0.x releases would be to upgrade to the latest 0.x before
> > making the jump to 1.0.
> >
> > Cheers,
> > Adam
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: [discuss] PropertyDescriptor name and displayName attributes

Joe Witt
I share Bryan's perspective.

On Fri, May 6, 2016 at 9:05 AM, Bryan Bende <[hidden email]> wrote:

> I might just be resistant to change, but I am still on the fence a little
> bit...
>
> In the past the idea has always been you start out with name, and if you
> later need to change what is displayed in the UI, then you add displayName
> after the fact.
>
> It sounds like the issue is that a lot of people don't know about
> displayName, so I am totally in favor of increasing awareness through
> documentation,
> but I'm struggling with telling people that they should set displayName as
> the default behavior.
>
> For code that is contributed to NiFi, I would expect the PMC/committer
> doing the review/merging to notice if an existing property name was being
> changed and point that out in the review.
> If it was someone else's custom NAR, or even made it through the review, I
> think what happens is that the flow starts up and the processor/component
> becomes invalid because it now has an unknown property.
> This is the same behavior when we remove a property from an existing
> processor and someone upgrades, and we deemed this acceptable behavior for
> that scenario.
>
> The internationalization aspect could possibly sell me more, but I think I
> would need someone to explain how internationalization would be
> implemented, and how setting the displayName helps.
> What Brandon described makes sense to me because it is something outside
> the Java code, which is different then saying all PropertyDescriptor
> instances need name and displayName.
>
> -Bryan
>
>
> On Fri, May 6, 2016 at 8:44 AM, Brandon DeVries <[hidden email]> wrote:
>
>> +1.  I think being able to move the displayName out of code an into config
>> / ResourceBundle will make it much easier to support i18n[1].  If no config
>> is provided, the name is the default... otherwise, the name displayed is
>> determined by the locale.  Updating the mock framework to complain about
>> the absence of a ResourceBundle will encourage adoption, and we'll
>> gradually work our way to not being English only.
>>
>> Brandon
>>
>> [1] https://docs.oracle.com/javase/tutorial/i18n/intro/quick.html
>>
>> On Thu, May 5, 2016 at 11:46 PM Adam Lamar <[hidden email]> wrote:
>>
>> > On Tue, May 3, 2016 at 12:09 PM, Andy LoPresto <[hidden email]>
>> > wrote:
>> >
>> > > As a result of some conversations and some varying feedback on PRs, I’d
>> > like
>> > > to discuss with the community an issue I see with PropertyDescriptor
>> name
>> > > and displayName attributes. I’ll describe the scenarios that cause
>> issues
>> > > and my proposed solution, and then solicit responses and other
>> > perspectives.
>> >
>> > Andy,
>> >
>> > I'd be +1 on this as well. I think the power of this approach will
>> > become more clear over time, and some of the automation will make it
>> > possible to more widely enforce.
>> >
>> > What do you think about a mixed mode where config reading code can
>> > fetch the property value with either the name or display name as the
>> > key, defaulting to the name if it is present? A sample read of
>> > flow.xml.gz might look like this:
>> >
>> > * Processor asks for value of MY_CONFIG_PROPERTY
>> > * Configuration code looks for key "my-config-property", returns if
>> present
>> > * Configuration code looks for key "My Config Property", returns if
>> present
>> > * On finding no valid key, configuration returns blank/default/null
>> > value (whatever is done today)
>> >
>> > On configuration write, the new attributes could be written as the
>> > normalized name (instead of display name), to allow processors that
>> > have made the switch to start using the normalized name field and
>> > start taking advantage of the new features around it (e.g.
>> > internationalization). Perhaps a disadvantage to this approach is that
>> > it auto-upgrades an existing flow.xml.gz, making it difficult to
>> > downgrade from (for example) 0.7 back to 0.6.
>> >
>> > A strategy like this (or something similar) might help speed adoption
>> > by making the process a bit smoother for existing flow.xml.gz files
>> > and easier to upgrade processors incrementally. At 1.0, display names
>> > as configuration keys could be dropped, and the upgrade path for users
>> > on old 0.x releases would be to upgrade to the latest 0.x before
>> > making the jump to 1.0.
>> >
>> > Cheers,
>> > Adam
>> >
>>
Reply | Threaded
Open this post in threaded view
|

Re: [discuss] PropertyDescriptor name and displayName attributes

Brandon DeVries
In reply to this post by Bryan Bende
Bryan,

The tutorial I linked to[1] has a really quick view of what implementing
i18n might look like[2].  We would probably need to add "country" and
"langauge" as entries in nifi.properties, use those to create a Locale, and
then set the display name with something like:

     ResourceBundle displayNames=
ResourceBundle.getBundle("DisplayNamesBundle", currentLocale);
     String i18nName = displayNames.getString(MyProcessor.class.toString());
     PropertyDescriptor.Builder().displayName(i18nName)...

Obviously we'd want to think this through all the way, but I think this
would be the general idea.

[1] https://docs.oracle.com/javase/tutorial/i18n/intro/quick.html
[2] https://docs.oracle.com/javase/tutorial/i18n/intro/after.html


On Fri, May 6, 2016 at 9:05 AM Bryan Bende <[hidden email]> wrote:

> I might just be resistant to change, but I am still on the fence a little
> bit...
>
> In the past the idea has always been you start out with name, and if you
> later need to change what is displayed in the UI, then you add displayName
> after the fact.
>
> It sounds like the issue is that a lot of people don't know about
> displayName, so I am totally in favor of increasing awareness through
> documentation,
> but I'm struggling with telling people that they should set displayName as
> the default behavior.
>
> For code that is contributed to NiFi, I would expect the PMC/committer
> doing the review/merging to notice if an existing property name was being
> changed and point that out in the review.
> If it was someone else's custom NAR, or even made it through the review, I
> think what happens is that the flow starts up and the processor/component
> becomes invalid because it now has an unknown property.
> This is the same behavior when we remove a property from an existing
> processor and someone upgrades, and we deemed this acceptable behavior for
> that scenario.
>
> The internationalization aspect could possibly sell me more, but I think I
> would need someone to explain how internationalization would be
> implemented, and how setting the displayName helps.
> What Brandon described makes sense to me because it is something outside
> the Java code, which is different then saying all PropertyDescriptor
> instances need name and displayName.
>
> -Bryan
>
>
> On Fri, May 6, 2016 at 8:44 AM, Brandon DeVries <[hidden email]> wrote:
>
> > +1.  I think being able to move the displayName out of code an into
> config
> > / ResourceBundle will make it much easier to support i18n[1].  If no
> config
> > is provided, the name is the default... otherwise, the name displayed is
> > determined by the locale.  Updating the mock framework to complain about
> > the absence of a ResourceBundle will encourage adoption, and we'll
> > gradually work our way to not being English only.
> >
> > Brandon
> >
> > [1] https://docs.oracle.com/javase/tutorial/i18n/intro/quick.html
> >
> > On Thu, May 5, 2016 at 11:46 PM Adam Lamar <[hidden email]> wrote:
> >
> > > On Tue, May 3, 2016 at 12:09 PM, Andy LoPresto <[hidden email]>
> > > wrote:
> > >
> > > > As a result of some conversations and some varying feedback on PRs,
> I’d
> > > like
> > > > to discuss with the community an issue I see with PropertyDescriptor
> > name
> > > > and displayName attributes. I’ll describe the scenarios that cause
> > issues
> > > > and my proposed solution, and then solicit responses and other
> > > perspectives.
> > >
> > > Andy,
> > >
> > > I'd be +1 on this as well. I think the power of this approach will
> > > become more clear over time, and some of the automation will make it
> > > possible to more widely enforce.
> > >
> > > What do you think about a mixed mode where config reading code can
> > > fetch the property value with either the name or display name as the
> > > key, defaulting to the name if it is present? A sample read of
> > > flow.xml.gz might look like this:
> > >
> > > * Processor asks for value of MY_CONFIG_PROPERTY
> > > * Configuration code looks for key "my-config-property", returns if
> > present
> > > * Configuration code looks for key "My Config Property", returns if
> > present
> > > * On finding no valid key, configuration returns blank/default/null
> > > value (whatever is done today)
> > >
> > > On configuration write, the new attributes could be written as the
> > > normalized name (instead of display name), to allow processors that
> > > have made the switch to start using the normalized name field and
> > > start taking advantage of the new features around it (e.g.
> > > internationalization). Perhaps a disadvantage to this approach is that
> > > it auto-upgrades an existing flow.xml.gz, making it difficult to
> > > downgrade from (for example) 0.7 back to 0.6.
> > >
> > > A strategy like this (or something similar) might help speed adoption
> > > by making the process a bit smoother for existing flow.xml.gz files
> > > and easier to upgrade processors incrementally. At 1.0, display names
> > > as configuration keys could be dropped, and the upgrade path for users
> > > on old 0.x releases would be to upgrade to the latest 0.x before
> > > making the jump to 1.0.
> > >
> > > Cheers,
> > > Adam
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: [discuss] PropertyDescriptor name and displayName attributes

Matt Burgess
In reply to this post by Joe Witt
Same here. Internationalization is often implemented as properties
files/resources, where you possibly load in a file based on the system
setting for Locale (like processor_names_en_US.properties). If we were
to do internationalization this way (i.e. a non-code based solution,
which is more flexible), then ironically displayName() might/should be
deprecated in favor of using the value of name() as the key in a
properties/lookup file; the corresponding value would be the
appropriate locale-specific "display name".

Brandon's links show this approach, I have seen this i18n approach on
other projects/products and it seems to work pretty well.

Regards,
Matt

On Fri, May 6, 2016 at 9:11 AM, Joe Witt <[hidden email]> wrote:

> I share Bryan's perspective.
>
> On Fri, May 6, 2016 at 9:05 AM, Bryan Bende <[hidden email]> wrote:
>> I might just be resistant to change, but I am still on the fence a little
>> bit...
>>
>> In the past the idea has always been you start out with name, and if you
>> later need to change what is displayed in the UI, then you add displayName
>> after the fact.
>>
>> It sounds like the issue is that a lot of people don't know about
>> displayName, so I am totally in favor of increasing awareness through
>> documentation,
>> but I'm struggling with telling people that they should set displayName as
>> the default behavior.
>>
>> For code that is contributed to NiFi, I would expect the PMC/committer
>> doing the review/merging to notice if an existing property name was being
>> changed and point that out in the review.
>> If it was someone else's custom NAR, or even made it through the review, I
>> think what happens is that the flow starts up and the processor/component
>> becomes invalid because it now has an unknown property.
>> This is the same behavior when we remove a property from an existing
>> processor and someone upgrades, and we deemed this acceptable behavior for
>> that scenario.
>>
>> The internationalization aspect could possibly sell me more, but I think I
>> would need someone to explain how internationalization would be
>> implemented, and how setting the displayName helps.
>> What Brandon described makes sense to me because it is something outside
>> the Java code, which is different then saying all PropertyDescriptor
>> instances need name and displayName.
>>
>> -Bryan
>>
>>
>> On Fri, May 6, 2016 at 8:44 AM, Brandon DeVries <[hidden email]> wrote:
>>
>>> +1.  I think being able to move the displayName out of code an into config
>>> / ResourceBundle will make it much easier to support i18n[1].  If no config
>>> is provided, the name is the default... otherwise, the name displayed is
>>> determined by the locale.  Updating the mock framework to complain about
>>> the absence of a ResourceBundle will encourage adoption, and we'll
>>> gradually work our way to not being English only.
>>>
>>> Brandon
>>>
>>> [1] https://docs.oracle.com/javase/tutorial/i18n/intro/quick.html
>>>
>>> On Thu, May 5, 2016 at 11:46 PM Adam Lamar <[hidden email]> wrote:
>>>
>>> > On Tue, May 3, 2016 at 12:09 PM, Andy LoPresto <[hidden email]>
>>> > wrote:
>>> >
>>> > > As a result of some conversations and some varying feedback on PRs, I’d
>>> > like
>>> > > to discuss with the community an issue I see with PropertyDescriptor
>>> name
>>> > > and displayName attributes. I’ll describe the scenarios that cause
>>> issues
>>> > > and my proposed solution, and then solicit responses and other
>>> > perspectives.
>>> >
>>> > Andy,
>>> >
>>> > I'd be +1 on this as well. I think the power of this approach will
>>> > become more clear over time, and some of the automation will make it
>>> > possible to more widely enforce.
>>> >
>>> > What do you think about a mixed mode where config reading code can
>>> > fetch the property value with either the name or display name as the
>>> > key, defaulting to the name if it is present? A sample read of
>>> > flow.xml.gz might look like this:
>>> >
>>> > * Processor asks for value of MY_CONFIG_PROPERTY
>>> > * Configuration code looks for key "my-config-property", returns if
>>> present
>>> > * Configuration code looks for key "My Config Property", returns if
>>> present
>>> > * On finding no valid key, configuration returns blank/default/null
>>> > value (whatever is done today)
>>> >
>>> > On configuration write, the new attributes could be written as the
>>> > normalized name (instead of display name), to allow processors that
>>> > have made the switch to start using the normalized name field and
>>> > start taking advantage of the new features around it (e.g.
>>> > internationalization). Perhaps a disadvantage to this approach is that
>>> > it auto-upgrades an existing flow.xml.gz, making it difficult to
>>> > downgrade from (for example) 0.7 back to 0.6.
>>> >
>>> > A strategy like this (or something similar) might help speed adoption
>>> > by making the process a bit smoother for existing flow.xml.gz files
>>> > and easier to upgrade processors incrementally. At 1.0, display names
>>> > as configuration keys could be dropped, and the upgrade path for users
>>> > on old 0.x releases would be to upgrade to the latest 0.x before
>>> > making the jump to 1.0.
>>> >
>>> > Cheers,
>>> > Adam
>>> >
>>>
Reply | Threaded
Open this post in threaded view
|

Re: [discuss] PropertyDescriptor name and displayName attributes

Brandon DeVries
+1.  I like that better.  Deprecate displayName(), and set it
"automatically" based on the locale from properties.  The name of the
property (which should never change) is the key into the ResourceBundle.

Brandon


On Fri, May 6, 2016 at 9:24 AM Matt Burgess <[hidden email]> wrote:

> Same here. Internationalization is often implemented as properties
> files/resources, where you possibly load in a file based on the system
> setting for Locale (like processor_names_en_US.properties). If we were
> to do internationalization this way (i.e. a non-code based solution,
> which is more flexible), then ironically displayName() might/should be
> deprecated in favor of using the value of name() as the key in a
> properties/lookup file; the corresponding value would be the
> appropriate locale-specific "display name".
>
> Brandon's links show this approach, I have seen this i18n approach on
> other projects/products and it seems to work pretty well.
>
> Regards,
> Matt
>
> On Fri, May 6, 2016 at 9:11 AM, Joe Witt <[hidden email]> wrote:
> > I share Bryan's perspective.
> >
> > On Fri, May 6, 2016 at 9:05 AM, Bryan Bende <[hidden email]> wrote:
> >> I might just be resistant to change, but I am still on the fence a
> little
> >> bit...
> >>
> >> In the past the idea has always been you start out with name, and if you
> >> later need to change what is displayed in the UI, then you add
> displayName
> >> after the fact.
> >>
> >> It sounds like the issue is that a lot of people don't know about
> >> displayName, so I am totally in favor of increasing awareness through
> >> documentation,
> >> but I'm struggling with telling people that they should set displayName
> as
> >> the default behavior.
> >>
> >> For code that is contributed to NiFi, I would expect the PMC/committer
> >> doing the review/merging to notice if an existing property name was
> being
> >> changed and point that out in the review.
> >> If it was someone else's custom NAR, or even made it through the
> review, I
> >> think what happens is that the flow starts up and the
> processor/component
> >> becomes invalid because it now has an unknown property.
> >> This is the same behavior when we remove a property from an existing
> >> processor and someone upgrades, and we deemed this acceptable behavior
> for
> >> that scenario.
> >>
> >> The internationalization aspect could possibly sell me more, but I
> think I
> >> would need someone to explain how internationalization would be
> >> implemented, and how setting the displayName helps.
> >> What Brandon described makes sense to me because it is something outside
> >> the Java code, which is different then saying all PropertyDescriptor
> >> instances need name and displayName.
> >>
> >> -Bryan
> >>
> >>
> >> On Fri, May 6, 2016 at 8:44 AM, Brandon DeVries <[hidden email]> wrote:
> >>
> >>> +1.  I think being able to move the displayName out of code an into
> config
> >>> / ResourceBundle will make it much easier to support i18n[1].  If no
> config
> >>> is provided, the name is the default... otherwise, the name displayed
> is
> >>> determined by the locale.  Updating the mock framework to complain
> about
> >>> the absence of a ResourceBundle will encourage adoption, and we'll
> >>> gradually work our way to not being English only.
> >>>
> >>> Brandon
> >>>
> >>> [1] https://docs.oracle.com/javase/tutorial/i18n/intro/quick.html
> >>>
> >>> On Thu, May 5, 2016 at 11:46 PM Adam Lamar <[hidden email]>
> wrote:
> >>>
> >>> > On Tue, May 3, 2016 at 12:09 PM, Andy LoPresto <[hidden email]
> >
> >>> > wrote:
> >>> >
> >>> > > As a result of some conversations and some varying feedback on
> PRs, I’d
> >>> > like
> >>> > > to discuss with the community an issue I see with
> PropertyDescriptor
> >>> name
> >>> > > and displayName attributes. I’ll describe the scenarios that cause
> >>> issues
> >>> > > and my proposed solution, and then solicit responses and other
> >>> > perspectives.
> >>> >
> >>> > Andy,
> >>> >
> >>> > I'd be +1 on this as well. I think the power of this approach will
> >>> > become more clear over time, and some of the automation will make it
> >>> > possible to more widely enforce.
> >>> >
> >>> > What do you think about a mixed mode where config reading code can
> >>> > fetch the property value with either the name or display name as the
> >>> > key, defaulting to the name if it is present? A sample read of
> >>> > flow.xml.gz might look like this:
> >>> >
> >>> > * Processor asks for value of MY_CONFIG_PROPERTY
> >>> > * Configuration code looks for key "my-config-property", returns if
> >>> present
> >>> > * Configuration code looks for key "My Config Property", returns if
> >>> present
> >>> > * On finding no valid key, configuration returns blank/default/null
> >>> > value (whatever is done today)
> >>> >
> >>> > On configuration write, the new attributes could be written as the
> >>> > normalized name (instead of display name), to allow processors that
> >>> > have made the switch to start using the normalized name field and
> >>> > start taking advantage of the new features around it (e.g.
> >>> > internationalization). Perhaps a disadvantage to this approach is
> that
> >>> > it auto-upgrades an existing flow.xml.gz, making it difficult to
> >>> > downgrade from (for example) 0.7 back to 0.6.
> >>> >
> >>> > A strategy like this (or something similar) might help speed adoption
> >>> > by making the process a bit smoother for existing flow.xml.gz files
> >>> > and easier to upgrade processors incrementally. At 1.0, display names
> >>> > as configuration keys could be dropped, and the upgrade path for
> users
> >>> > on old 0.x releases would be to upgrade to the latest 0.x before
> >>> > making the jump to 1.0.
> >>> >
> >>> > Cheers,
> >>> > Adam
> >>> >
> >>>
>
Reply | Threaded
Open this post in threaded view
|

Re: [discuss] PropertyDescriptor name and displayName attributes

Joe Witt
Definitely on board with the idea that the 'name' will be the key to a
resource bundle.  It does imply such names will need to follow
necessary conventions to be valid resource bundle keys.

However, in the spirit of always thinking about the developer path to
productivity I am hopeful we can come up with a nice way to not
require them to setup a resource bundle.

The idea being that the following order of operations/thought would exist:

1) How can I provide a new property to this processor?
Answer: Add a property descriptor and set the name.  This name will be
used to refer to the property descriptor whenever serialized/saving
the config and it will be rendered through the REST API and thus made
available as the property name in the UI.

2) Oh snap.  I wish I had used a different name because I've found a
better way to communicate intent to the user.  How do I do this?
Answer: Go ahead and set displayName.  NiFi will continue to use the
'name' for serialization/config saving but will use the displayName
for what is shown to the user in the UI.

3) I would like to support locale sensitive representations of my
property name.  How can I do this?
Answer: Add a resource bundle with entries for your property 'name'
value.  This means the resource bundle needs to exist and your
property 'name' must adhere to resource bundle key naming requirements
[1].  If this is supplied and can be looked up then this will be used
and otherwise will fallback to using displayName value if present and
otherwise will fallback to using the value of 'name'.

And in any event we still need to better document/articulate this
model as the root of this thread was that we hadn't effectively
communicated the existence of displayName.  I agree this discussion
ended up getting us to a great place though as we should all strive to
support internationalization.

With an approach like this I am onboard.  I think this balances our
goals of having a simple to use API but also allows those who want to
support multiple locales to do so cleanly.

Thanks
Joe

[1] https://docs.oracle.com/javase/tutorial/i18n/resbundle/propfile.html

On Fri, May 6, 2016 at 9:33 AM, Brandon DeVries <[hidden email]> wrote:

> +1.  I like that better.  Deprecate displayName(), and set it
> "automatically" based on the locale from properties.  The name of the
> property (which should never change) is the key into the ResourceBundle.
>
> Brandon
>
>
> On Fri, May 6, 2016 at 9:24 AM Matt Burgess <[hidden email]> wrote:
>
>> Same here. Internationalization is often implemented as properties
>> files/resources, where you possibly load in a file based on the system
>> setting for Locale (like processor_names_en_US.properties). If we were
>> to do internationalization this way (i.e. a non-code based solution,
>> which is more flexible), then ironically displayName() might/should be
>> deprecated in favor of using the value of name() as the key in a
>> properties/lookup file; the corresponding value would be the
>> appropriate locale-specific "display name".
>>
>> Brandon's links show this approach, I have seen this i18n approach on
>> other projects/products and it seems to work pretty well.
>>
>> Regards,
>> Matt
>>
>> On Fri, May 6, 2016 at 9:11 AM, Joe Witt <[hidden email]> wrote:
>> > I share Bryan's perspective.
>> >
>> > On Fri, May 6, 2016 at 9:05 AM, Bryan Bende <[hidden email]> wrote:
>> >> I might just be resistant to change, but I am still on the fence a
>> little
>> >> bit...
>> >>
>> >> In the past the idea has always been you start out with name, and if you
>> >> later need to change what is displayed in the UI, then you add
>> displayName
>> >> after the fact.
>> >>
>> >> It sounds like the issue is that a lot of people don't know about
>> >> displayName, so I am totally in favor of increasing awareness through
>> >> documentation,
>> >> but I'm struggling with telling people that they should set displayName
>> as
>> >> the default behavior.
>> >>
>> >> For code that is contributed to NiFi, I would expect the PMC/committer
>> >> doing the review/merging to notice if an existing property name was
>> being
>> >> changed and point that out in the review.
>> >> If it was someone else's custom NAR, or even made it through the
>> review, I
>> >> think what happens is that the flow starts up and the
>> processor/component
>> >> becomes invalid because it now has an unknown property.
>> >> This is the same behavior when we remove a property from an existing
>> >> processor and someone upgrades, and we deemed this acceptable behavior
>> for
>> >> that scenario.
>> >>
>> >> The internationalization aspect could possibly sell me more, but I
>> think I
>> >> would need someone to explain how internationalization would be
>> >> implemented, and how setting the displayName helps.
>> >> What Brandon described makes sense to me because it is something outside
>> >> the Java code, which is different then saying all PropertyDescriptor
>> >> instances need name and displayName.
>> >>
>> >> -Bryan
>> >>
>> >>
>> >> On Fri, May 6, 2016 at 8:44 AM, Brandon DeVries <[hidden email]> wrote:
>> >>
>> >>> +1.  I think being able to move the displayName out of code an into
>> config
>> >>> / ResourceBundle will make it much easier to support i18n[1].  If no
>> config
>> >>> is provided, the name is the default... otherwise, the name displayed
>> is
>> >>> determined by the locale.  Updating the mock framework to complain
>> about
>> >>> the absence of a ResourceBundle will encourage adoption, and we'll
>> >>> gradually work our way to not being English only.
>> >>>
>> >>> Brandon
>> >>>
>> >>> [1] https://docs.oracle.com/javase/tutorial/i18n/intro/quick.html
>> >>>
>> >>> On Thu, May 5, 2016 at 11:46 PM Adam Lamar <[hidden email]>
>> wrote:
>> >>>
>> >>> > On Tue, May 3, 2016 at 12:09 PM, Andy LoPresto <[hidden email]
>> >
>> >>> > wrote:
>> >>> >
>> >>> > > As a result of some conversations and some varying feedback on
>> PRs, I’d
>> >>> > like
>> >>> > > to discuss with the community an issue I see with
>> PropertyDescriptor
>> >>> name
>> >>> > > and displayName attributes. I’ll describe the scenarios that cause
>> >>> issues
>> >>> > > and my proposed solution, and then solicit responses and other
>> >>> > perspectives.
>> >>> >
>> >>> > Andy,
>> >>> >
>> >>> > I'd be +1 on this as well. I think the power of this approach will
>> >>> > become more clear over time, and some of the automation will make it
>> >>> > possible to more widely enforce.
>> >>> >
>> >>> > What do you think about a mixed mode where config reading code can
>> >>> > fetch the property value with either the name or display name as the
>> >>> > key, defaulting to the name if it is present? A sample read of
>> >>> > flow.xml.gz might look like this:
>> >>> >
>> >>> > * Processor asks for value of MY_CONFIG_PROPERTY
>> >>> > * Configuration code looks for key "my-config-property", returns if
>> >>> present
>> >>> > * Configuration code looks for key "My Config Property", returns if
>> >>> present
>> >>> > * On finding no valid key, configuration returns blank/default/null
>> >>> > value (whatever is done today)
>> >>> >
>> >>> > On configuration write, the new attributes could be written as the
>> >>> > normalized name (instead of display name), to allow processors that
>> >>> > have made the switch to start using the normalized name field and
>> >>> > start taking advantage of the new features around it (e.g.
>> >>> > internationalization). Perhaps a disadvantage to this approach is
>> that
>> >>> > it auto-upgrades an existing flow.xml.gz, making it difficult to
>> >>> > downgrade from (for example) 0.7 back to 0.6.
>> >>> >
>> >>> > A strategy like this (or something similar) might help speed adoption
>> >>> > by making the process a bit smoother for existing flow.xml.gz files
>> >>> > and easier to upgrade processors incrementally. At 1.0, display names
>> >>> > as configuration keys could be dropped, and the upgrade path for
>> users
>> >>> > on old 0.x releases would be to upgrade to the latest 0.x before
>> >>> > making the jump to 1.0.
>> >>> >
>> >>> > Cheers,
>> >>> > Adam
>> >>> >
>> >>>
>>
Reply | Threaded
Open this post in threaded view
|

Re: [discuss] PropertyDescriptor name and displayName attributes

Oleg Zhurakousky
I think the the main source of confusion (as it was for me) is the ‘name’ name itself.
Basically the ‘name’ today is really an identifier of the property and therefore could/should never change while 'displayName' is volatile.
Obviously changing “name’ to “id” is out of the question as it would break practically everything. But from the documentation perspective may be we should consider documenting that ‘name’ corresponds to a unique and perpetual identifier of the property that would also be displayed unless override by ‘displayName’ while such override would only affect the display characteristics of the property and not its identification.

Cheers
Oleg

> On May 6, 2016, at 10:25 AM, Joe Witt <[hidden email]> wrote:
>
> Definitely on board with the idea that the 'name' will be the key to a
> resource bundle.  It does imply such names will need to follow
> necessary conventions to be valid resource bundle keys.
>
> However, in the spirit of always thinking about the developer path to
> productivity I am hopeful we can come up with a nice way to not
> require them to setup a resource bundle.
>
> The idea being that the following order of operations/thought would exist:
>
> 1) How can I provide a new property to this processor?
> Answer: Add a property descriptor and set the name.  This name will be
> used to refer to the property descriptor whenever serialized/saving
> the config and it will be rendered through the REST API and thus made
> available as the property name in the UI.
>
> 2) Oh snap.  I wish I had used a different name because I've found a
> better way to communicate intent to the user.  How do I do this?
> Answer: Go ahead and set displayName.  NiFi will continue to use the
> 'name' for serialization/config saving but will use the displayName
> for what is shown to the user in the UI.
>
> 3) I would like to support locale sensitive representations of my
> property name.  How can I do this?
> Answer: Add a resource bundle with entries for your property 'name'
> value.  This means the resource bundle needs to exist and your
> property 'name' must adhere to resource bundle key naming requirements
> [1].  If this is supplied and can be looked up then this will be used
> and otherwise will fallback to using displayName value if present and
> otherwise will fallback to using the value of 'name'.
>
> And in any event we still need to better document/articulate this
> model as the root of this thread was that we hadn't effectively
> communicated the existence of displayName.  I agree this discussion
> ended up getting us to a great place though as we should all strive to
> support internationalization.
>
> With an approach like this I am onboard.  I think this balances our
> goals of having a simple to use API but also allows those who want to
> support multiple locales to do so cleanly.
>
> Thanks
> Joe
>
> [1] https://docs.oracle.com/javase/tutorial/i18n/resbundle/propfile.html
>
> On Fri, May 6, 2016 at 9:33 AM, Brandon DeVries <[hidden email]> wrote:
>> +1.  I like that better.  Deprecate displayName(), and set it
>> "automatically" based on the locale from properties.  The name of the
>> property (which should never change) is the key into the ResourceBundle.
>>
>> Brandon
>>
>>
>> On Fri, May 6, 2016 at 9:24 AM Matt Burgess <[hidden email]> wrote:
>>
>>> Same here. Internationalization is often implemented as properties
>>> files/resources, where you possibly load in a file based on the system
>>> setting for Locale (like processor_names_en_US.properties). If we were
>>> to do internationalization this way (i.e. a non-code based solution,
>>> which is more flexible), then ironically displayName() might/should be
>>> deprecated in favor of using the value of name() as the key in a
>>> properties/lookup file; the corresponding value would be the
>>> appropriate locale-specific "display name".
>>>
>>> Brandon's links show this approach, I have seen this i18n approach on
>>> other projects/products and it seems to work pretty well.
>>>
>>> Regards,
>>> Matt
>>>
>>> On Fri, May 6, 2016 at 9:11 AM, Joe Witt <[hidden email]> wrote:
>>>> I share Bryan's perspective.
>>>>
>>>> On Fri, May 6, 2016 at 9:05 AM, Bryan Bende <[hidden email]> wrote:
>>>>> I might just be resistant to change, but I am still on the fence a
>>> little
>>>>> bit...
>>>>>
>>>>> In the past the idea has always been you start out with name, and if you
>>>>> later need to change what is displayed in the UI, then you add
>>> displayName
>>>>> after the fact.
>>>>>
>>>>> It sounds like the issue is that a lot of people don't know about
>>>>> displayName, so I am totally in favor of increasing awareness through
>>>>> documentation,
>>>>> but I'm struggling with telling people that they should set displayName
>>> as
>>>>> the default behavior.
>>>>>
>>>>> For code that is contributed to NiFi, I would expect the PMC/committer
>>>>> doing the review/merging to notice if an existing property name was
>>> being
>>>>> changed and point that out in the review.
>>>>> If it was someone else's custom NAR, or even made it through the
>>> review, I
>>>>> think what happens is that the flow starts up and the
>>> processor/component
>>>>> becomes invalid because it now has an unknown property.
>>>>> This is the same behavior when we remove a property from an existing
>>>>> processor and someone upgrades, and we deemed this acceptable behavior
>>> for
>>>>> that scenario.
>>>>>
>>>>> The internationalization aspect could possibly sell me more, but I
>>> think I
>>>>> would need someone to explain how internationalization would be
>>>>> implemented, and how setting the displayName helps.
>>>>> What Brandon described makes sense to me because it is something outside
>>>>> the Java code, which is different then saying all PropertyDescriptor
>>>>> instances need name and displayName.
>>>>>
>>>>> -Bryan
>>>>>
>>>>>
>>>>> On Fri, May 6, 2016 at 8:44 AM, Brandon DeVries <[hidden email]> wrote:
>>>>>
>>>>>> +1.  I think being able to move the displayName out of code an into
>>> config
>>>>>> / ResourceBundle will make it much easier to support i18n[1].  If no
>>> config
>>>>>> is provided, the name is the default... otherwise, the name displayed
>>> is
>>>>>> determined by the locale.  Updating the mock framework to complain
>>> about
>>>>>> the absence of a ResourceBundle will encourage adoption, and we'll
>>>>>> gradually work our way to not being English only.
>>>>>>
>>>>>> Brandon
>>>>>>
>>>>>> [1] https://docs.oracle.com/javase/tutorial/i18n/intro/quick.html
>>>>>>
>>>>>> On Thu, May 5, 2016 at 11:46 PM Adam Lamar <[hidden email]>
>>> wrote:
>>>>>>
>>>>>>> On Tue, May 3, 2016 at 12:09 PM, Andy LoPresto <[hidden email]
>>>>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> As a result of some conversations and some varying feedback on
>>> PRs, I’d
>>>>>>> like
>>>>>>>> to discuss with the community an issue I see with
>>> PropertyDescriptor
>>>>>> name
>>>>>>>> and displayName attributes. I’ll describe the scenarios that cause
>>>>>> issues
>>>>>>>> and my proposed solution, and then solicit responses and other
>>>>>>> perspectives.
>>>>>>>
>>>>>>> Andy,
>>>>>>>
>>>>>>> I'd be +1 on this as well. I think the power of this approach will
>>>>>>> become more clear over time, and some of the automation will make it
>>>>>>> possible to more widely enforce.
>>>>>>>
>>>>>>> What do you think about a mixed mode where config reading code can
>>>>>>> fetch the property value with either the name or display name as the
>>>>>>> key, defaulting to the name if it is present? A sample read of
>>>>>>> flow.xml.gz might look like this:
>>>>>>>
>>>>>>> * Processor asks for value of MY_CONFIG_PROPERTY
>>>>>>> * Configuration code looks for key "my-config-property", returns if
>>>>>> present
>>>>>>> * Configuration code looks for key "My Config Property", returns if
>>>>>> present
>>>>>>> * On finding no valid key, configuration returns blank/default/null
>>>>>>> value (whatever is done today)
>>>>>>>
>>>>>>> On configuration write, the new attributes could be written as the
>>>>>>> normalized name (instead of display name), to allow processors that
>>>>>>> have made the switch to start using the normalized name field and
>>>>>>> start taking advantage of the new features around it (e.g.
>>>>>>> internationalization). Perhaps a disadvantage to this approach is
>>> that
>>>>>>> it auto-upgrades an existing flow.xml.gz, making it difficult to
>>>>>>> downgrade from (for example) 0.7 back to 0.6.
>>>>>>>
>>>>>>> A strategy like this (or something similar) might help speed adoption
>>>>>>> by making the process a bit smoother for existing flow.xml.gz files
>>>>>>> and easier to upgrade processors incrementally. At 1.0, display names
>>>>>>> as configuration keys could be dropped, and the upgrade path for
>>> users
>>>>>>> on old 0.x releases would be to upgrade to the latest 0.x before
>>>>>>> making the jump to 1.0.
>>>>>>>
>>>>>>> Cheers,
>>>>>>> Adam
>>>>>>>
>>>>>>
>>>
>

Reply | Threaded
Open this post in threaded view
|

Re: [discuss] PropertyDescriptor name and displayName attributes

Brandon DeVries
In reply to this post by Joe Witt
I guess my only objection is, do we really need Option 2?  If all you want
to do is provide a name, fine.  But, if you want to change the name (for
better communicating the purpose to the user, be that in English, French,
or Esperanto) option 3 provides the mechanism for that.  It's not as easy
for the developer as option 2, but it has the benefit of starting the
process of creating resource bundles for different languages (and
potentially deprecating a semi-confusing option).  This means that someone
wanting to add support for a different language (e.g. french) has a lower
barrier to entry, in that they can just find the "displayNames.properties"
file, and put a "displayNames_fr_FR.properties next to it.  Ultimately its
a trade off of conveniences... the developer wanting to change a property
name vs. a potential contributor wanting to improve language support.  I
guess I would argue that this shouldn't be that hard for a developer to
figure out, and lean in favor of encouraging language contributions.

Ultimately, it isn't' that strong an objection though, and it does sound
like this is heading in the right direction...

Brandon

On Fri, May 6, 2016 at 10:25 AM Joe Witt <[hidden email]> wrote:

> Definitely on board with the idea that the 'name' will be the key to a
> resource bundle.  It does imply such names will need to follow
> necessary conventions to be valid resource bundle keys.
>
> However, in the spirit of always thinking about the developer path to
> productivity I am hopeful we can come up with a nice way to not
> require them to setup a resource bundle.
>
> The idea being that the following order of operations/thought would exist:
>
> 1) How can I provide a new property to this processor?
> Answer: Add a property descriptor and set the name.  This name will be
> used to refer to the property descriptor whenever serialized/saving
> the config and it will be rendered through the REST API and thus made
> available as the property name in the UI.
>
> 2) Oh snap.  I wish I had used a different name because I've found a
> better way to communicate intent to the user.  How do I do this?
> Answer: Go ahead and set displayName.  NiFi will continue to use the
> 'name' for serialization/config saving but will use the displayName
> for what is shown to the user in the UI.
>
> 3) I would like to support locale sensitive representations of my
> property name.  How can I do this?
> Answer: Add a resource bundle with entries for your property 'name'
> value.  This means the resource bundle needs to exist and your
> property 'name' must adhere to resource bundle key naming requirements
> [1].  If this is supplied and can be looked up then this will be used
> and otherwise will fallback to using displayName value if present and
> otherwise will fallback to using the value of 'name'.
>
> And in any event we still need to better document/articulate this
> model as the root of this thread was that we hadn't effectively
> communicated the existence of displayName.  I agree this discussion
> ended up getting us to a great place though as we should all strive to
> support internationalization.
>
> With an approach like this I am onboard.  I think this balances our
> goals of having a simple to use API but also allows those who want to
> support multiple locales to do so cleanly.
>
> Thanks
> Joe
>
> [1] https://docs.oracle.com/javase/tutorial/i18n/resbundle/propfile.html
>
> On Fri, May 6, 2016 at 9:33 AM, Brandon DeVries <[hidden email]> wrote:
> > +1.  I like that better.  Deprecate displayName(), and set it
> > "automatically" based on the locale from properties.  The name of the
> > property (which should never change) is the key into the ResourceBundle.
> >
> > Brandon
> >
> >
> > On Fri, May 6, 2016 at 9:24 AM Matt Burgess <[hidden email]> wrote:
> >
> >> Same here. Internationalization is often implemented as properties
> >> files/resources, where you possibly load in a file based on the system
> >> setting for Locale (like processor_names_en_US.properties). If we were
> >> to do internationalization this way (i.e. a non-code based solution,
> >> which is more flexible), then ironically displayName() might/should be
> >> deprecated in favor of using the value of name() as the key in a
> >> properties/lookup file; the corresponding value would be the
> >> appropriate locale-specific "display name".
> >>
> >> Brandon's links show this approach, I have seen this i18n approach on
> >> other projects/products and it seems to work pretty well.
> >>
> >> Regards,
> >> Matt
> >>
> >> On Fri, May 6, 2016 at 9:11 AM, Joe Witt <[hidden email]> wrote:
> >> > I share Bryan's perspective.
> >> >
> >> > On Fri, May 6, 2016 at 9:05 AM, Bryan Bende <[hidden email]> wrote:
> >> >> I might just be resistant to change, but I am still on the fence a
> >> little
> >> >> bit...
> >> >>
> >> >> In the past the idea has always been you start out with name, and if
> you
> >> >> later need to change what is displayed in the UI, then you add
> >> displayName
> >> >> after the fact.
> >> >>
> >> >> It sounds like the issue is that a lot of people don't know about
> >> >> displayName, so I am totally in favor of increasing awareness through
> >> >> documentation,
> >> >> but I'm struggling with telling people that they should set
> displayName
> >> as
> >> >> the default behavior.
> >> >>
> >> >> For code that is contributed to NiFi, I would expect the
> PMC/committer
> >> >> doing the review/merging to notice if an existing property name was
> >> being
> >> >> changed and point that out in the review.
> >> >> If it was someone else's custom NAR, or even made it through the
> >> review, I
> >> >> think what happens is that the flow starts up and the
> >> processor/component
> >> >> becomes invalid because it now has an unknown property.
> >> >> This is the same behavior when we remove a property from an existing
> >> >> processor and someone upgrades, and we deemed this acceptable
> behavior
> >> for
> >> >> that scenario.
> >> >>
> >> >> The internationalization aspect could possibly sell me more, but I
> >> think I
> >> >> would need someone to explain how internationalization would be
> >> >> implemented, and how setting the displayName helps.
> >> >> What Brandon described makes sense to me because it is something
> outside
> >> >> the Java code, which is different then saying all PropertyDescriptor
> >> >> instances need name and displayName.
> >> >>
> >> >> -Bryan
> >> >>
> >> >>
> >> >> On Fri, May 6, 2016 at 8:44 AM, Brandon DeVries <[hidden email]> wrote:
> >> >>
> >> >>> +1.  I think being able to move the displayName out of code an into
> >> config
> >> >>> / ResourceBundle will make it much easier to support i18n[1].  If no
> >> config
> >> >>> is provided, the name is the default... otherwise, the name
> displayed
> >> is
> >> >>> determined by the locale.  Updating the mock framework to complain
> >> about
> >> >>> the absence of a ResourceBundle will encourage adoption, and we'll
> >> >>> gradually work our way to not being English only.
> >> >>>
> >> >>> Brandon
> >> >>>
> >> >>> [1] https://docs.oracle.com/javase/tutorial/i18n/intro/quick.html
> >> >>>
> >> >>> On Thu, May 5, 2016 at 11:46 PM Adam Lamar <[hidden email]>
> >> wrote:
> >> >>>
> >> >>> > On Tue, May 3, 2016 at 12:09 PM, Andy LoPresto <
> [hidden email]
> >> >
> >> >>> > wrote:
> >> >>> >
> >> >>> > > As a result of some conversations and some varying feedback on
> >> PRs, I’d
> >> >>> > like
> >> >>> > > to discuss with the community an issue I see with
> >> PropertyDescriptor
> >> >>> name
> >> >>> > > and displayName attributes. I’ll describe the scenarios that
> cause
> >> >>> issues
> >> >>> > > and my proposed solution, and then solicit responses and other
> >> >>> > perspectives.
> >> >>> >
> >> >>> > Andy,
> >> >>> >
> >> >>> > I'd be +1 on this as well. I think the power of this approach will
> >> >>> > become more clear over time, and some of the automation will make
> it
> >> >>> > possible to more widely enforce.
> >> >>> >
> >> >>> > What do you think about a mixed mode where config reading code can
> >> >>> > fetch the property value with either the name or display name as
> the
> >> >>> > key, defaulting to the name if it is present? A sample read of
> >> >>> > flow.xml.gz might look like this:
> >> >>> >
> >> >>> > * Processor asks for value of MY_CONFIG_PROPERTY
> >> >>> > * Configuration code looks for key "my-config-property", returns
> if
> >> >>> present
> >> >>> > * Configuration code looks for key "My Config Property", returns
> if
> >> >>> present
> >> >>> > * On finding no valid key, configuration returns
> blank/default/null
> >> >>> > value (whatever is done today)
> >> >>> >
> >> >>> > On configuration write, the new attributes could be written as the
> >> >>> > normalized name (instead of display name), to allow processors
> that
> >> >>> > have made the switch to start using the normalized name field and
> >> >>> > start taking advantage of the new features around it (e.g.
> >> >>> > internationalization). Perhaps a disadvantage to this approach is
> >> that
> >> >>> > it auto-upgrades an existing flow.xml.gz, making it difficult to
> >> >>> > downgrade from (for example) 0.7 back to 0.6.
> >> >>> >
> >> >>> > A strategy like this (or something similar) might help speed
> adoption
> >> >>> > by making the process a bit smoother for existing flow.xml.gz
> files
> >> >>> > and easier to upgrade processors incrementally. At 1.0, display
> names
> >> >>> > as configuration keys could be dropped, and the upgrade path for
> >> users
> >> >>> > on old 0.x releases would be to upgrade to the latest 0.x before
> >> >>> > making the jump to 1.0.
> >> >>> >
> >> >>> > Cheers,
> >> >>> > Adam
> >> >>> >
> >> >>>
> >>
>
Reply | Threaded
Open this post in threaded view
|

Re: [discuss] PropertyDescriptor name and displayName attributes

Brandon DeVries
In reply to this post by Oleg Zhurakousky
With 1.0.0, could we deprecate name() and replace it with id()?  Under the
hood name() would still call id() (which would just be the current name()
renamed...) until we finally pull the plug on it, but in the meantime it
might encourage developers to understand what name / id really means.

Brandon

On Fri, May 6, 2016 at 10:34 AM Oleg Zhurakousky <
[hidden email]> wrote:

> I think the the main source of confusion (as it was for me) is the ‘name’
> name itself.
> Basically the ‘name’ today is really an identifier of the property and
> therefore could/should never change while 'displayName' is volatile.
> Obviously changing “name’ to “id” is out of the question as it would break
> practically everything. But from the documentation perspective may be we
> should consider documenting that ‘name’ corresponds to a unique and
> perpetual identifier of the property that would also be displayed unless
> override by ‘displayName’ while such override would only affect the display
> characteristics of the property and not its identification.
>
> Cheers
> Oleg
>
> > On May 6, 2016, at 10:25 AM, Joe Witt <[hidden email]> wrote:
> >
> > Definitely on board with the idea that the 'name' will be the key to a
> > resource bundle.  It does imply such names will need to follow
> > necessary conventions to be valid resource bundle keys.
> >
> > However, in the spirit of always thinking about the developer path to
> > productivity I am hopeful we can come up with a nice way to not
> > require them to setup a resource bundle.
> >
> > The idea being that the following order of operations/thought would
> exist:
> >
> > 1) How can I provide a new property to this processor?
> > Answer: Add a property descriptor and set the name.  This name will be
> > used to refer to the property descriptor whenever serialized/saving
> > the config and it will be rendered through the REST API and thus made
> > available as the property name in the UI.
> >
> > 2) Oh snap.  I wish I had used a different name because I've found a
> > better way to communicate intent to the user.  How do I do this?
> > Answer: Go ahead and set displayName.  NiFi will continue to use the
> > 'name' for serialization/config saving but will use the displayName
> > for what is shown to the user in the UI.
> >
> > 3) I would like to support locale sensitive representations of my
> > property name.  How can I do this?
> > Answer: Add a resource bundle with entries for your property 'name'
> > value.  This means the resource bundle needs to exist and your
> > property 'name' must adhere to resource bundle key naming requirements
> > [1].  If this is supplied and can be looked up then this will be used
> > and otherwise will fallback to using displayName value if present and
> > otherwise will fallback to using the value of 'name'.
> >
> > And in any event we still need to better document/articulate this
> > model as the root of this thread was that we hadn't effectively
> > communicated the existence of displayName.  I agree this discussion
> > ended up getting us to a great place though as we should all strive to
> > support internationalization.
> >
> > With an approach like this I am onboard.  I think this balances our
> > goals of having a simple to use API but also allows those who want to
> > support multiple locales to do so cleanly.
> >
> > Thanks
> > Joe
> >
> > [1] https://docs.oracle.com/javase/tutorial/i18n/resbundle/propfile.html
> >
> > On Fri, May 6, 2016 at 9:33 AM, Brandon DeVries <[hidden email]> wrote:
> >> +1.  I like that better.  Deprecate displayName(), and set it
> >> "automatically" based on the locale from properties.  The name of the
> >> property (which should never change) is the key into the ResourceBundle.
> >>
> >> Brandon
> >>
> >>
> >> On Fri, May 6, 2016 at 9:24 AM Matt Burgess <[hidden email]>
> wrote:
> >>
> >>> Same here. Internationalization is often implemented as properties
> >>> files/resources, where you possibly load in a file based on the system
> >>> setting for Locale (like processor_names_en_US.properties). If we were
> >>> to do internationalization this way (i.e. a non-code based solution,
> >>> which is more flexible), then ironically displayName() might/should be
> >>> deprecated in favor of using the value of name() as the key in a
> >>> properties/lookup file; the corresponding value would be the
> >>> appropriate locale-specific "display name".
> >>>
> >>> Brandon's links show this approach, I have seen this i18n approach on
> >>> other projects/products and it seems to work pretty well.
> >>>
> >>> Regards,
> >>> Matt
> >>>
> >>> On Fri, May 6, 2016 at 9:11 AM, Joe Witt <[hidden email]> wrote:
> >>>> I share Bryan's perspective.
> >>>>
> >>>> On Fri, May 6, 2016 at 9:05 AM, Bryan Bende <[hidden email]> wrote:
> >>>>> I might just be resistant to change, but I am still on the fence a
> >>> little
> >>>>> bit...
> >>>>>
> >>>>> In the past the idea has always been you start out with name, and if
> you
> >>>>> later need to change what is displayed in the UI, then you add
> >>> displayName
> >>>>> after the fact.
> >>>>>
> >>>>> It sounds like the issue is that a lot of people don't know about
> >>>>> displayName, so I am totally in favor of increasing awareness through
> >>>>> documentation,
> >>>>> but I'm struggling with telling people that they should set
> displayName
> >>> as
> >>>>> the default behavior.
> >>>>>
> >>>>> For code that is contributed to NiFi, I would expect the
> PMC/committer
> >>>>> doing the review/merging to notice if an existing property name was
> >>> being
> >>>>> changed and point that out in the review.
> >>>>> If it was someone else's custom NAR, or even made it through the
> >>> review, I
> >>>>> think what happens is that the flow starts up and the
> >>> processor/component
> >>>>> becomes invalid because it now has an unknown property.
> >>>>> This is the same behavior when we remove a property from an existing
> >>>>> processor and someone upgrades, and we deemed this acceptable
> behavior
> >>> for
> >>>>> that scenario.
> >>>>>
> >>>>> The internationalization aspect could possibly sell me more, but I
> >>> think I
> >>>>> would need someone to explain how internationalization would be
> >>>>> implemented, and how setting the displayName helps.
> >>>>> What Brandon described makes sense to me because it is something
> outside
> >>>>> the Java code, which is different then saying all PropertyDescriptor
> >>>>> instances need name and displayName.
> >>>>>
> >>>>> -Bryan
> >>>>>
> >>>>>
> >>>>> On Fri, May 6, 2016 at 8:44 AM, Brandon DeVries <[hidden email]> wrote:
> >>>>>
> >>>>>> +1.  I think being able to move the displayName out of code an into
> >>> config
> >>>>>> / ResourceBundle will make it much easier to support i18n[1].  If no
> >>> config
> >>>>>> is provided, the name is the default... otherwise, the name
> displayed
> >>> is
> >>>>>> determined by the locale.  Updating the mock framework to complain
> >>> about
> >>>>>> the absence of a ResourceBundle will encourage adoption, and we'll
> >>>>>> gradually work our way to not being English only.
> >>>>>>
> >>>>>> Brandon
> >>>>>>
> >>>>>> [1] https://docs.oracle.com/javase/tutorial/i18n/intro/quick.html
> >>>>>>
> >>>>>> On Thu, May 5, 2016 at 11:46 PM Adam Lamar <[hidden email]>
> >>> wrote:
> >>>>>>
> >>>>>>> On Tue, May 3, 2016 at 12:09 PM, Andy LoPresto <
> [hidden email]
> >>>>
> >>>>>>> wrote:
> >>>>>>>
> >>>>>>>> As a result of some conversations and some varying feedback on
> >>> PRs, I’d
> >>>>>>> like
> >>>>>>>> to discuss with the community an issue I see with
> >>> PropertyDescriptor
> >>>>>> name
> >>>>>>>> and displayName attributes. I’ll describe the scenarios that cause
> >>>>>> issues
> >>>>>>>> and my proposed solution, and then solicit responses and other
> >>>>>>> perspectives.
> >>>>>>>
> >>>>>>> Andy,
> >>>>>>>
> >>>>>>> I'd be +1 on this as well. I think the power of this approach will
> >>>>>>> become more clear over time, and some of the automation will make
> it
> >>>>>>> possible to more widely enforce.
> >>>>>>>
> >>>>>>> What do you think about a mixed mode where config reading code can
> >>>>>>> fetch the property value with either the name or display name as
> the
> >>>>>>> key, defaulting to the name if it is present? A sample read of
> >>>>>>> flow.xml.gz might look like this:
> >>>>>>>
> >>>>>>> * Processor asks for value of MY_CONFIG_PROPERTY
> >>>>>>> * Configuration code looks for key "my-config-property", returns if
> >>>>>> present
> >>>>>>> * Configuration code looks for key "My Config Property", returns if
> >>>>>> present
> >>>>>>> * On finding no valid key, configuration returns blank/default/null
> >>>>>>> value (whatever is done today)
> >>>>>>>
> >>>>>>> On configuration write, the new attributes could be written as the
> >>>>>>> normalized name (instead of display name), to allow processors that
> >>>>>>> have made the switch to start using the normalized name field and
> >>>>>>> start taking advantage of the new features around it (e.g.
> >>>>>>> internationalization). Perhaps a disadvantage to this approach is
> >>> that
> >>>>>>> it auto-upgrades an existing flow.xml.gz, making it difficult to
> >>>>>>> downgrade from (for example) 0.7 back to 0.6.
> >>>>>>>
> >>>>>>> A strategy like this (or something similar) might help speed
> adoption
> >>>>>>> by making the process a bit smoother for existing flow.xml.gz files
> >>>>>>> and easier to upgrade processors incrementally. At 1.0, display
> names
> >>>>>>> as configuration keys could be dropped, and the upgrade path for
> >>> users
> >>>>>>> on old 0.x releases would be to upgrade to the latest 0.x before
> >>>>>>> making the jump to 1.0.
> >>>>>>>
> >>>>>>> Cheers,
> >>>>>>> Adam
> >>>>>>>
> >>>>>>
> >>>
> >
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [discuss] PropertyDescriptor name and displayName attributes

Joe Witt
In reply to this post by Brandon DeVries
Brandon

Understood.  I'd say I lean more towards providing 1, 2, and 3 because
allows us to be flexible in what we accept and conservative in what we
do.  By that I mean we as a community can go ahead and enforce that we
basically just adopt 3 and use RTC to help enforce that.  But that
from an API and doing examples perspective we allow 1 and 2 as well.
I don't think it adds much weight but does add flexibility.

Like you, I think I'm ok either way.  This was a good discussion and
ending up with supporting internationalization and flexibility in
displayed name while retaining configuration sanity - is a win.

Thanks
Joe

On Fri, May 6, 2016 at 10:45 AM, Brandon DeVries <[hidden email]> wrote:

> I guess my only objection is, do we really need Option 2?  If all you want
> to do is provide a name, fine.  But, if you want to change the name (for
> better communicating the purpose to the user, be that in English, French,
> or Esperanto) option 3 provides the mechanism for that.  It's not as easy
> for the developer as option 2, but it has the benefit of starting the
> process of creating resource bundles for different languages (and
> potentially deprecating a semi-confusing option).  This means that someone
> wanting to add support for a different language (e.g. french) has a lower
> barrier to entry, in that they can just find the "displayNames.properties"
> file, and put a "displayNames_fr_FR.properties next to it.  Ultimately its
> a trade off of conveniences... the developer wanting to change a property
> name vs. a potential contributor wanting to improve language support.  I
> guess I would argue that this shouldn't be that hard for a developer to
> figure out, and lean in favor of encouraging language contributions.
>
> Ultimately, it isn't' that strong an objection though, and it does sound
> like this is heading in the right direction...
>
> Brandon
>
> On Fri, May 6, 2016 at 10:25 AM Joe Witt <[hidden email]> wrote:
>
>> Definitely on board with the idea that the 'name' will be the key to a
>> resource bundle.  It does imply such names will need to follow
>> necessary conventions to be valid resource bundle keys.
>>
>> However, in the spirit of always thinking about the developer path to
>> productivity I am hopeful we can come up with a nice way to not
>> require them to setup a resource bundle.
>>
>> The idea being that the following order of operations/thought would exist:
>>
>> 1) How can I provide a new property to this processor?
>> Answer: Add a property descriptor and set the name.  This name will be
>> used to refer to the property descriptor whenever serialized/saving
>> the config and it will be rendered through the REST API and thus made
>> available as the property name in the UI.
>>
>> 2) Oh snap.  I wish I had used a different name because I've found a
>> better way to communicate intent to the user.  How do I do this?
>> Answer: Go ahead and set displayName.  NiFi will continue to use the
>> 'name' for serialization/config saving but will use the displayName
>> for what is shown to the user in the UI.
>>
>> 3) I would like to support locale sensitive representations of my
>> property name.  How can I do this?
>> Answer: Add a resource bundle with entries for your property 'name'
>> value.  This means the resource bundle needs to exist and your
>> property 'name' must adhere to resource bundle key naming requirements
>> [1].  If this is supplied and can be looked up then this will be used
>> and otherwise will fallback to using displayName value if present and
>> otherwise will fallback to using the value of 'name'.
>>
>> And in any event we still need to better document/articulate this
>> model as the root of this thread was that we hadn't effectively
>> communicated the existence of displayName.  I agree this discussion
>> ended up getting us to a great place though as we should all strive to
>> support internationalization.
>>
>> With an approach like this I am onboard.  I think this balances our
>> goals of having a simple to use API but also allows those who want to
>> support multiple locales to do so cleanly.
>>
>> Thanks
>> Joe
>>
>> [1] https://docs.oracle.com/javase/tutorial/i18n/resbundle/propfile.html
>>
>> On Fri, May 6, 2016 at 9:33 AM, Brandon DeVries <[hidden email]> wrote:
>> > +1.  I like that better.  Deprecate displayName(), and set it
>> > "automatically" based on the locale from properties.  The name of the
>> > property (which should never change) is the key into the ResourceBundle.
>> >
>> > Brandon
>> >
>> >
>> > On Fri, May 6, 2016 at 9:24 AM Matt Burgess <[hidden email]> wrote:
>> >
>> >> Same here. Internationalization is often implemented as properties
>> >> files/resources, where you possibly load in a file based on the system
>> >> setting for Locale (like processor_names_en_US.properties). If we were
>> >> to do internationalization this way (i.e. a non-code based solution,
>> >> which is more flexible), then ironically displayName() might/should be
>> >> deprecated in favor of using the value of name() as the key in a
>> >> properties/lookup file; the corresponding value would be the
>> >> appropriate locale-specific "display name".
>> >>
>> >> Brandon's links show this approach, I have seen this i18n approach on
>> >> other projects/products and it seems to work pretty well.
>> >>
>> >> Regards,
>> >> Matt
>> >>
>> >> On Fri, May 6, 2016 at 9:11 AM, Joe Witt <[hidden email]> wrote:
>> >> > I share Bryan's perspective.
>> >> >
>> >> > On Fri, May 6, 2016 at 9:05 AM, Bryan Bende <[hidden email]> wrote:
>> >> >> I might just be resistant to change, but I am still on the fence a
>> >> little
>> >> >> bit...
>> >> >>
>> >> >> In the past the idea has always been you start out with name, and if
>> you
>> >> >> later need to change what is displayed in the UI, then you add
>> >> displayName
>> >> >> after the fact.
>> >> >>
>> >> >> It sounds like the issue is that a lot of people don't know about
>> >> >> displayName, so I am totally in favor of increasing awareness through
>> >> >> documentation,
>> >> >> but I'm struggling with telling people that they should set
>> displayName
>> >> as
>> >> >> the default behavior.
>> >> >>
>> >> >> For code that is contributed to NiFi, I would expect the
>> PMC/committer
>> >> >> doing the review/merging to notice if an existing property name was
>> >> being
>> >> >> changed and point that out in the review.
>> >> >> If it was someone else's custom NAR, or even made it through the
>> >> review, I
>> >> >> think what happens is that the flow starts up and the
>> >> processor/component
>> >> >> becomes invalid because it now has an unknown property.
>> >> >> This is the same behavior when we remove a property from an existing
>> >> >> processor and someone upgrades, and we deemed this acceptable
>> behavior
>> >> for
>> >> >> that scenario.
>> >> >>
>> >> >> The internationalization aspect could possibly sell me more, but I
>> >> think I
>> >> >> would need someone to explain how internationalization would be
>> >> >> implemented, and how setting the displayName helps.
>> >> >> What Brandon described makes sense to me because it is something
>> outside
>> >> >> the Java code, which is different then saying all PropertyDescriptor
>> >> >> instances need name and displayName.
>> >> >>
>> >> >> -Bryan
>> >> >>
>> >> >>
>> >> >> On Fri, May 6, 2016 at 8:44 AM, Brandon DeVries <[hidden email]> wrote:
>> >> >>
>> >> >>> +1.  I think being able to move the displayName out of code an into
>> >> config
>> >> >>> / ResourceBundle will make it much easier to support i18n[1].  If no
>> >> config
>> >> >>> is provided, the name is the default... otherwise, the name
>> displayed
>> >> is
>> >> >>> determined by the locale.  Updating the mock framework to complain
>> >> about
>> >> >>> the absence of a ResourceBundle will encourage adoption, and we'll
>> >> >>> gradually work our way to not being English only.
>> >> >>>
>> >> >>> Brandon
>> >> >>>
>> >> >>> [1] https://docs.oracle.com/javase/tutorial/i18n/intro/quick.html
>> >> >>>
>> >> >>> On Thu, May 5, 2016 at 11:46 PM Adam Lamar <[hidden email]>
>> >> wrote:
>> >> >>>
>> >> >>> > On Tue, May 3, 2016 at 12:09 PM, Andy LoPresto <
>> [hidden email]
>> >> >
>> >> >>> > wrote:
>> >> >>> >
>> >> >>> > > As a result of some conversations and some varying feedback on
>> >> PRs, I’d
>> >> >>> > like
>> >> >>> > > to discuss with the community an issue I see with
>> >> PropertyDescriptor
>> >> >>> name
>> >> >>> > > and displayName attributes. I’ll describe the scenarios that
>> cause
>> >> >>> issues
>> >> >>> > > and my proposed solution, and then solicit responses and other
>> >> >>> > perspectives.
>> >> >>> >
>> >> >>> > Andy,
>> >> >>> >
>> >> >>> > I'd be +1 on this as well. I think the power of this approach will
>> >> >>> > become more clear over time, and some of the automation will make
>> it
>> >> >>> > possible to more widely enforce.
>> >> >>> >
>> >> >>> > What do you think about a mixed mode where config reading code can
>> >> >>> > fetch the property value with either the name or display name as
>> the
>> >> >>> > key, defaulting to the name if it is present? A sample read of
>> >> >>> > flow.xml.gz might look like this:
>> >> >>> >
>> >> >>> > * Processor asks for value of MY_CONFIG_PROPERTY
>> >> >>> > * Configuration code looks for key "my-config-property", returns
>> if
>> >> >>> present
>> >> >>> > * Configuration code looks for key "My Config Property", returns
>> if
>> >> >>> present
>> >> >>> > * On finding no valid key, configuration returns
>> blank/default/null
>> >> >>> > value (whatever is done today)
>> >> >>> >
>> >> >>> > On configuration write, the new attributes could be written as the
>> >> >>> > normalized name (instead of display name), to allow processors
>> that
>> >> >>> > have made the switch to start using the normalized name field and
>> >> >>> > start taking advantage of the new features around it (e.g.
>> >> >>> > internationalization). Perhaps a disadvantage to this approach is
>> >> that
>> >> >>> > it auto-upgrades an existing flow.xml.gz, making it difficult to
>> >> >>> > downgrade from (for example) 0.7 back to 0.6.
>> >> >>> >
>> >> >>> > A strategy like this (or something similar) might help speed
>> adoption
>> >> >>> > by making the process a bit smoother for existing flow.xml.gz
>> files
>> >> >>> > and easier to upgrade processors incrementally. At 1.0, display
>> names
>> >> >>> > as configuration keys could be dropped, and the upgrade path for
>> >> users
>> >> >>> > on old 0.x releases would be to upgrade to the latest 0.x before
>> >> >>> > making the jump to 1.0.
>> >> >>> >
>> >> >>> > Cheers,
>> >> >>> > Adam
>> >> >>> >
>> >> >>>
>> >>
>>
Reply | Threaded
Open this post in threaded view
|

Re: [discuss] PropertyDescriptor name and displayName attributes

Joe Witt
In reply to this post by Brandon DeVries
I don't think we gain much by going from 'name' to 'identifier'.  The
other parts of this conversation have much stronger value I think.

Thanks
Joe

On Fri, May 6, 2016 at 10:50 AM, Brandon DeVries <[hidden email]> wrote:

> With 1.0.0, could we deprecate name() and replace it with id()?  Under the
> hood name() would still call id() (which would just be the current name()
> renamed...) until we finally pull the plug on it, but in the meantime it
> might encourage developers to understand what name / id really means.
>
> Brandon
>
> On Fri, May 6, 2016 at 10:34 AM Oleg Zhurakousky <
> [hidden email]> wrote:
>
>> I think the the main source of confusion (as it was for me) is the ‘name’
>> name itself.
>> Basically the ‘name’ today is really an identifier of the property and
>> therefore could/should never change while 'displayName' is volatile.
>> Obviously changing “name’ to “id” is out of the question as it would break
>> practically everything. But from the documentation perspective may be we
>> should consider documenting that ‘name’ corresponds to a unique and
>> perpetual identifier of the property that would also be displayed unless
>> override by ‘displayName’ while such override would only affect the display
>> characteristics of the property and not its identification.
>>
>> Cheers
>> Oleg
>>
>> > On May 6, 2016, at 10:25 AM, Joe Witt <[hidden email]> wrote:
>> >
>> > Definitely on board with the idea that the 'name' will be the key to a
>> > resource bundle.  It does imply such names will need to follow
>> > necessary conventions to be valid resource bundle keys.
>> >
>> > However, in the spirit of always thinking about the developer path to
>> > productivity I am hopeful we can come up with a nice way to not
>> > require them to setup a resource bundle.
>> >
>> > The idea being that the following order of operations/thought would
>> exist:
>> >
>> > 1) How can I provide a new property to this processor?
>> > Answer: Add a property descriptor and set the name.  This name will be
>> > used to refer to the property descriptor whenever serialized/saving
>> > the config and it will be rendered through the REST API and thus made
>> > available as the property name in the UI.
>> >
>> > 2) Oh snap.  I wish I had used a different name because I've found a
>> > better way to communicate intent to the user.  How do I do this?
>> > Answer: Go ahead and set displayName.  NiFi will continue to use the
>> > 'name' for serialization/config saving but will use the displayName
>> > for what is shown to the user in the UI.
>> >
>> > 3) I would like to support locale sensitive representations of my
>> > property name.  How can I do this?
>> > Answer: Add a resource bundle with entries for your property 'name'
>> > value.  This means the resource bundle needs to exist and your
>> > property 'name' must adhere to resource bundle key naming requirements
>> > [1].  If this is supplied and can be looked up then this will be used
>> > and otherwise will fallback to using displayName value if present and
>> > otherwise will fallback to using the value of 'name'.
>> >
>> > And in any event we still need to better document/articulate this
>> > model as the root of this thread was that we hadn't effectively
>> > communicated the existence of displayName.  I agree this discussion
>> > ended up getting us to a great place though as we should all strive to
>> > support internationalization.
>> >
>> > With an approach like this I am onboard.  I think this balances our
>> > goals of having a simple to use API but also allows those who want to
>> > support multiple locales to do so cleanly.
>> >
>> > Thanks
>> > Joe
>> >
>> > [1] https://docs.oracle.com/javase/tutorial/i18n/resbundle/propfile.html
>> >
>> > On Fri, May 6, 2016 at 9:33 AM, Brandon DeVries <[hidden email]> wrote:
>> >> +1.  I like that better.  Deprecate displayName(), and set it
>> >> "automatically" based on the locale from properties.  The name of the
>> >> property (which should never change) is the key into the ResourceBundle.
>> >>
>> >> Brandon
>> >>
>> >>
>> >> On Fri, May 6, 2016 at 9:24 AM Matt Burgess <[hidden email]>
>> wrote:
>> >>
>> >>> Same here. Internationalization is often implemented as properties
>> >>> files/resources, where you possibly load in a file based on the system
>> >>> setting for Locale (like processor_names_en_US.properties). If we were
>> >>> to do internationalization this way (i.e. a non-code based solution,
>> >>> which is more flexible), then ironically displayName() might/should be
>> >>> deprecated in favor of using the value of name() as the key in a
>> >>> properties/lookup file; the corresponding value would be the
>> >>> appropriate locale-specific "display name".
>> >>>
>> >>> Brandon's links show this approach, I have seen this i18n approach on
>> >>> other projects/products and it seems to work pretty well.
>> >>>
>> >>> Regards,
>> >>> Matt
>> >>>
>> >>> On Fri, May 6, 2016 at 9:11 AM, Joe Witt <[hidden email]> wrote:
>> >>>> I share Bryan's perspective.
>> >>>>
>> >>>> On Fri, May 6, 2016 at 9:05 AM, Bryan Bende <[hidden email]> wrote:
>> >>>>> I might just be resistant to change, but I am still on the fence a
>> >>> little
>> >>>>> bit...
>> >>>>>
>> >>>>> In the past the idea has always been you start out with name, and if
>> you
>> >>>>> later need to change what is displayed in the UI, then you add
>> >>> displayName
>> >>>>> after the fact.
>> >>>>>
>> >>>>> It sounds like the issue is that a lot of people don't know about
>> >>>>> displayName, so I am totally in favor of increasing awareness through
>> >>>>> documentation,
>> >>>>> but I'm struggling with telling people that they should set
>> displayName
>> >>> as
>> >>>>> the default behavior.
>> >>>>>
>> >>>>> For code that is contributed to NiFi, I would expect the
>> PMC/committer
>> >>>>> doing the review/merging to notice if an existing property name was
>> >>> being
>> >>>>> changed and point that out in the review.
>> >>>>> If it was someone else's custom NAR, or even made it through the
>> >>> review, I
>> >>>>> think what happens is that the flow starts up and the
>> >>> processor/component
>> >>>>> becomes invalid because it now has an unknown property.
>> >>>>> This is the same behavior when we remove a property from an existing
>> >>>>> processor and someone upgrades, and we deemed this acceptable
>> behavior
>> >>> for
>> >>>>> that scenario.
>> >>>>>
>> >>>>> The internationalization aspect could possibly sell me more, but I
>> >>> think I
>> >>>>> would need someone to explain how internationalization would be
>> >>>>> implemented, and how setting the displayName helps.
>> >>>>> What Brandon described makes sense to me because it is something
>> outside
>> >>>>> the Java code, which is different then saying all PropertyDescriptor
>> >>>>> instances need name and displayName.
>> >>>>>
>> >>>>> -Bryan
>> >>>>>
>> >>>>>
>> >>>>> On Fri, May 6, 2016 at 8:44 AM, Brandon DeVries <[hidden email]> wrote:
>> >>>>>
>> >>>>>> +1.  I think being able to move the displayName out of code an into
>> >>> config
>> >>>>>> / ResourceBundle will make it much easier to support i18n[1].  If no
>> >>> config
>> >>>>>> is provided, the name is the default... otherwise, the name
>> displayed
>> >>> is
>> >>>>>> determined by the locale.  Updating the mock framework to complain
>> >>> about
>> >>>>>> the absence of a ResourceBundle will encourage adoption, and we'll
>> >>>>>> gradually work our way to not being English only.
>> >>>>>>
>> >>>>>> Brandon
>> >>>>>>
>> >>>>>> [1] https://docs.oracle.com/javase/tutorial/i18n/intro/quick.html
>> >>>>>>
>> >>>>>> On Thu, May 5, 2016 at 11:46 PM Adam Lamar <[hidden email]>
>> >>> wrote:
>> >>>>>>
>> >>>>>>> On Tue, May 3, 2016 at 12:09 PM, Andy LoPresto <
>> [hidden email]
>> >>>>
>> >>>>>>> wrote:
>> >>>>>>>
>> >>>>>>>> As a result of some conversations and some varying feedback on
>> >>> PRs, I’d
>> >>>>>>> like
>> >>>>>>>> to discuss with the community an issue I see with
>> >>> PropertyDescriptor
>> >>>>>> name
>> >>>>>>>> and displayName attributes. I’ll describe the scenarios that cause
>> >>>>>> issues
>> >>>>>>>> and my proposed solution, and then solicit responses and other
>> >>>>>>> perspectives.
>> >>>>>>>
>> >>>>>>> Andy,
>> >>>>>>>
>> >>>>>>> I'd be +1 on this as well. I think the power of this approach will
>> >>>>>>> become more clear over time, and some of the automation will make
>> it
>> >>>>>>> possible to more widely enforce.
>> >>>>>>>
>> >>>>>>> What do you think about a mixed mode where config reading code can
>> >>>>>>> fetch the property value with either the name or display name as
>> the
>> >>>>>>> key, defaulting to the name if it is present? A sample read of
>> >>>>>>> flow.xml.gz might look like this:
>> >>>>>>>
>> >>>>>>> * Processor asks for value of MY_CONFIG_PROPERTY
>> >>>>>>> * Configuration code looks for key "my-config-property", returns if
>> >>>>>> present
>> >>>>>>> * Configuration code looks for key "My Config Property", returns if
>> >>>>>> present
>> >>>>>>> * On finding no valid key, configuration returns blank/default/null
>> >>>>>>> value (whatever is done today)
>> >>>>>>>
>> >>>>>>> On configuration write, the new attributes could be written as the
>> >>>>>>> normalized name (instead of display name), to allow processors that
>> >>>>>>> have made the switch to start using the normalized name field and
>> >>>>>>> start taking advantage of the new features around it (e.g.
>> >>>>>>> internationalization). Perhaps a disadvantage to this approach is
>> >>> that
>> >>>>>>> it auto-upgrades an existing flow.xml.gz, making it difficult to
>> >>>>>>> downgrade from (for example) 0.7 back to 0.6.
>> >>>>>>>
>> >>>>>>> A strategy like this (or something similar) might help speed
>> adoption
>> >>>>>>> by making the process a bit smoother for existing flow.xml.gz files
>> >>>>>>> and easier to upgrade processors incrementally. At 1.0, display
>> names
>> >>>>>>> as configuration keys could be dropped, and the upgrade path for
>> >>> users
>> >>>>>>> on old 0.x releases would be to upgrade to the latest 0.x before
>> >>>>>>> making the jump to 1.0.
>> >>>>>>>
>> >>>>>>> Cheers,
>> >>>>>>> Adam
>> >>>>>>>
>> >>>>>>
>> >>>
>> >
>>
>>
Reply | Threaded
Open this post in threaded view
|

Re: [discuss] PropertyDescriptor name and displayName attributes

Brandon DeVries
In reply to this post by Joe Witt
Joe,

Alright, I buy that.  As a community, we try not to do #2 (stop
laughing...). But we still allow someone writing / maintaining custom
components to do so without figuring out how to set up a resource bundle.
+1.

Brandon

On Fri, May 6, 2016 at 10:51 AM Joe Witt <[hidden email]> wrote:

> Brandon
>
> Understood.  I'd say I lean more towards providing 1, 2, and 3 because
> allows us to be flexible in what we accept and conservative in what we
> do.  By that I mean we as a community can go ahead and enforce that we
> basically just adopt 3 and use RTC to help enforce that.  But that
> from an API and doing examples perspective we allow 1 and 2 as well.
> I don't think it adds much weight but does add flexibility.
>
> Like you, I think I'm ok either way.  This was a good discussion and
> ending up with supporting internationalization and flexibility in
> displayed name while retaining configuration sanity - is a win.
>
> Thanks
> Joe
>
> On Fri, May 6, 2016 at 10:45 AM, Brandon DeVries <[hidden email]> wrote:
> > I guess my only objection is, do we really need Option 2?  If all you
> want
> > to do is provide a name, fine.  But, if you want to change the name (for
> > better communicating the purpose to the user, be that in English, French,
> > or Esperanto) option 3 provides the mechanism for that.  It's not as easy
> > for the developer as option 2, but it has the benefit of starting the
> > process of creating resource bundles for different languages (and
> > potentially deprecating a semi-confusing option).  This means that
> someone
> > wanting to add support for a different language (e.g. french) has a lower
> > barrier to entry, in that they can just find the
> "displayNames.properties"
> > file, and put a "displayNames_fr_FR.properties next to it.  Ultimately
> its
> > a trade off of conveniences... the developer wanting to change a property
> > name vs. a potential contributor wanting to improve language support.  I
> > guess I would argue that this shouldn't be that hard for a developer to
> > figure out, and lean in favor of encouraging language contributions.
> >
> > Ultimately, it isn't' that strong an objection though, and it does sound
> > like this is heading in the right direction...
> >
> > Brandon
> >
> > On Fri, May 6, 2016 at 10:25 AM Joe Witt <[hidden email]> wrote:
> >
> >> Definitely on board with the idea that the 'name' will be the key to a
> >> resource bundle.  It does imply such names will need to follow
> >> necessary conventions to be valid resource bundle keys.
> >>
> >> However, in the spirit of always thinking about the developer path to
> >> productivity I am hopeful we can come up with a nice way to not
> >> require them to setup a resource bundle.
> >>
> >> The idea being that the following order of operations/thought would
> exist:
> >>
> >> 1) How can I provide a new property to this processor?
> >> Answer: Add a property descriptor and set the name.  This name will be
> >> used to refer to the property descriptor whenever serialized/saving
> >> the config and it will be rendered through the REST API and thus made
> >> available as the property name in the UI.
> >>
> >> 2) Oh snap.  I wish I had used a different name because I've found a
> >> better way to communicate intent to the user.  How do I do this?
> >> Answer: Go ahead and set displayName.  NiFi will continue to use the
> >> 'name' for serialization/config saving but will use the displayName
> >> for what is shown to the user in the UI.
> >>
> >> 3) I would like to support locale sensitive representations of my
> >> property name.  How can I do this?
> >> Answer: Add a resource bundle with entries for your property 'name'
> >> value.  This means the resource bundle needs to exist and your
> >> property 'name' must adhere to resource bundle key naming requirements
> >> [1].  If this is supplied and can be looked up then this will be used
> >> and otherwise will fallback to using displayName value if present and
> >> otherwise will fallback to using the value of 'name'.
> >>
> >> And in any event we still need to better document/articulate this
> >> model as the root of this thread was that we hadn't effectively
> >> communicated the existence of displayName.  I agree this discussion
> >> ended up getting us to a great place though as we should all strive to
> >> support internationalization.
> >>
> >> With an approach like this I am onboard.  I think this balances our
> >> goals of having a simple to use API but also allows those who want to
> >> support multiple locales to do so cleanly.
> >>
> >> Thanks
> >> Joe
> >>
> >> [1]
> https://docs.oracle.com/javase/tutorial/i18n/resbundle/propfile.html
> >>
> >> On Fri, May 6, 2016 at 9:33 AM, Brandon DeVries <[hidden email]> wrote:
> >> > +1.  I like that better.  Deprecate displayName(), and set it
> >> > "automatically" based on the locale from properties.  The name of the
> >> > property (which should never change) is the key into the
> ResourceBundle.
> >> >
> >> > Brandon
> >> >
> >> >
> >> > On Fri, May 6, 2016 at 9:24 AM Matt Burgess <[hidden email]>
> wrote:
> >> >
> >> >> Same here. Internationalization is often implemented as properties
> >> >> files/resources, where you possibly load in a file based on the
> system
> >> >> setting for Locale (like processor_names_en_US.properties). If we
> were
> >> >> to do internationalization this way (i.e. a non-code based solution,
> >> >> which is more flexible), then ironically displayName() might/should
> be
> >> >> deprecated in favor of using the value of name() as the key in a
> >> >> properties/lookup file; the corresponding value would be the
> >> >> appropriate locale-specific "display name".
> >> >>
> >> >> Brandon's links show this approach, I have seen this i18n approach on
> >> >> other projects/products and it seems to work pretty well.
> >> >>
> >> >> Regards,
> >> >> Matt
> >> >>
> >> >> On Fri, May 6, 2016 at 9:11 AM, Joe Witt <[hidden email]> wrote:
> >> >> > I share Bryan's perspective.
> >> >> >
> >> >> > On Fri, May 6, 2016 at 9:05 AM, Bryan Bende <[hidden email]>
> wrote:
> >> >> >> I might just be resistant to change, but I am still on the fence a
> >> >> little
> >> >> >> bit...
> >> >> >>
> >> >> >> In the past the idea has always been you start out with name, and
> if
> >> you
> >> >> >> later need to change what is displayed in the UI, then you add
> >> >> displayName
> >> >> >> after the fact.
> >> >> >>
> >> >> >> It sounds like the issue is that a lot of people don't know about
> >> >> >> displayName, so I am totally in favor of increasing awareness
> through
> >> >> >> documentation,
> >> >> >> but I'm struggling with telling people that they should set
> >> displayName
> >> >> as
> >> >> >> the default behavior.
> >> >> >>
> >> >> >> For code that is contributed to NiFi, I would expect the
> >> PMC/committer
> >> >> >> doing the review/merging to notice if an existing property name
> was
> >> >> being
> >> >> >> changed and point that out in the review.
> >> >> >> If it was someone else's custom NAR, or even made it through the
> >> >> review, I
> >> >> >> think what happens is that the flow starts up and the
> >> >> processor/component
> >> >> >> becomes invalid because it now has an unknown property.
> >> >> >> This is the same behavior when we remove a property from an
> existing
> >> >> >> processor and someone upgrades, and we deemed this acceptable
> >> behavior
> >> >> for
> >> >> >> that scenario.
> >> >> >>
> >> >> >> The internationalization aspect could possibly sell me more, but I
> >> >> think I
> >> >> >> would need someone to explain how internationalization would be
> >> >> >> implemented, and how setting the displayName helps.
> >> >> >> What Brandon described makes sense to me because it is something
> >> outside
> >> >> >> the Java code, which is different then saying all
> PropertyDescriptor
> >> >> >> instances need name and displayName.
> >> >> >>
> >> >> >> -Bryan
> >> >> >>
> >> >> >>
> >> >> >> On Fri, May 6, 2016 at 8:44 AM, Brandon DeVries <[hidden email]>
> wrote:
> >> >> >>
> >> >> >>> +1.  I think being able to move the displayName out of code an
> into
> >> >> config
> >> >> >>> / ResourceBundle will make it much easier to support i18n[1].
> If no
> >> >> config
> >> >> >>> is provided, the name is the default... otherwise, the name
> >> displayed
> >> >> is
> >> >> >>> determined by the locale.  Updating the mock framework to
> complain
> >> >> about
> >> >> >>> the absence of a ResourceBundle will encourage adoption, and
> we'll
> >> >> >>> gradually work our way to not being English only.
> >> >> >>>
> >> >> >>> Brandon
> >> >> >>>
> >> >> >>> [1]
> https://docs.oracle.com/javase/tutorial/i18n/intro/quick.html
> >> >> >>>
> >> >> >>> On Thu, May 5, 2016 at 11:46 PM Adam Lamar <[hidden email]
> >
> >> >> wrote:
> >> >> >>>
> >> >> >>> > On Tue, May 3, 2016 at 12:09 PM, Andy LoPresto <
> >> [hidden email]
> >> >> >
> >> >> >>> > wrote:
> >> >> >>> >
> >> >> >>> > > As a result of some conversations and some varying feedback
> on
> >> >> PRs, I’d
> >> >> >>> > like
> >> >> >>> > > to discuss with the community an issue I see with
> >> >> PropertyDescriptor
> >> >> >>> name
> >> >> >>> > > and displayName attributes. I’ll describe the scenarios that
> >> cause
> >> >> >>> issues
> >> >> >>> > > and my proposed solution, and then solicit responses and
> other
> >> >> >>> > perspectives.
> >> >> >>> >
> >> >> >>> > Andy,
> >> >> >>> >
> >> >> >>> > I'd be +1 on this as well. I think the power of this approach
> will
> >> >> >>> > become more clear over time, and some of the automation will
> make
> >> it
> >> >> >>> > possible to more widely enforce.
> >> >> >>> >
> >> >> >>> > What do you think about a mixed mode where config reading code
> can
> >> >> >>> > fetch the property value with either the name or display name
> as
> >> the
> >> >> >>> > key, defaulting to the name if it is present? A sample read of
> >> >> >>> > flow.xml.gz might look like this:
> >> >> >>> >
> >> >> >>> > * Processor asks for value of MY_CONFIG_PROPERTY
> >> >> >>> > * Configuration code looks for key "my-config-property",
> returns
> >> if
> >> >> >>> present
> >> >> >>> > * Configuration code looks for key "My Config Property",
> returns
> >> if
> >> >> >>> present
> >> >> >>> > * On finding no valid key, configuration returns
> >> blank/default/null
> >> >> >>> > value (whatever is done today)
> >> >> >>> >
> >> >> >>> > On configuration write, the new attributes could be written as
> the
> >> >> >>> > normalized name (instead of display name), to allow processors
> >> that
> >> >> >>> > have made the switch to start using the normalized name field
> and
> >> >> >>> > start taking advantage of the new features around it (e.g.
> >> >> >>> > internationalization). Perhaps a disadvantage to this approach
> is
> >> >> that
> >> >> >>> > it auto-upgrades an existing flow.xml.gz, making it difficult
> to
> >> >> >>> > downgrade from (for example) 0.7 back to 0.6.
> >> >> >>> >
> >> >> >>> > A strategy like this (or something similar) might help speed
> >> adoption
> >> >> >>> > by making the process a bit smoother for existing flow.xml.gz
> >> files
> >> >> >>> > and easier to upgrade processors incrementally. At 1.0, display
> >> names
> >> >> >>> > as configuration keys could be dropped, and the upgrade path
> for
> >> >> users
> >> >> >>> > on old 0.x releases would be to upgrade to the latest 0.x
> before
> >> >> >>> > making the jump to 1.0.
> >> >> >>> >
> >> >> >>> > Cheers,
> >> >> >>> > Adam
> >> >> >>> >
> >> >> >>>
> >> >>
> >>
>
Reply | Threaded
Open this post in threaded view
|

Re: [discuss] PropertyDescriptor name and displayName attributes

Andy LoPresto-2
Joe, 

My concern with the ordered scenarios you listed is that I believe the level of awareness you describe will still cause difficulties based on the name value. 

Consider the scenario where I added the ability to encrypt or decrypt using a raw key with the EncryptContent processor [1]. I had to add a new property, and I’ve made inline comments below:

1) How can I provide a new property to this processor?
Answer: Add a property descriptor and set the name.  This name will be
used to refer to the property descriptor whenever serialized/saving
the config and it will be rendered through the REST API and thus made
available as the property name in the UI.

In this case, I chose “raw-key-hex” as the name, following the convention in this processor. I used “Raw Key (Hex)” as the displayName. If I had been adding this to a new processor or one without existing properties to demonstrate convention, I would have only known about “name”, and would have provided “Raw Key (Hex)” as the name, as that is what I want to be displayed in the UI. 


2) Oh snap.  I wish I had used a different name because I've found a
better way to communicate intent to the user.  How do I do this?
Answer: Go ahead and set displayName.  NiFi will continue to use the
'name' for serialization/config saving but will use the displayName
for what is shown to the user in the UI.

Let’s say that I realize non-crypto people don’t understand “Raw Key (Hex)” and I want to change it to “32 or 64 character key in hexadecimal format”. I learn about displayName after trying to change the name directly results in backward compatibility issues. 


3) I would like to support locale sensitive representations of my
property name.  How can I do this?
Answer: Add a resource bundle with entries for your property 'name'
value.  This means the resource bundle needs to exist and your
property 'name' must adhere to resource bundle key naming requirements
[1].  If this is supplied and can be looked up then this will be used
and otherwise will fallback to using displayName value if present and
otherwise will fallback to using the value of 'name'.

I decide I would like to internationalize my processor and support Spanish as an additional language. I configure a resource bundle with the phrase “Clave de cifrado (formato hexadecimal)”. What is the “key” I need to assign this value to? It’s the original name value, or “Raw Key (Hex)”. At this point, the name does not adhere to the key naming constraints [2] (technically spaces can be escaped but it requires additional effort), and I have no way to modify this without breaking the object resolution in the flow.xml.gz. 

I maintain that in an ideal world, providing the name and displayName values from the beginning establishes a clear and consistent convention that separates object resolution and UI display and presents a cohesive example for any follow-on properties declared in the processor. To me, this drives “community” because it provides a clear example for new developers, so they don’t get comments on their first PR saying “Oh, there is an issue when using a spaced- or quoted- name value that will cause problems down the road but unless you read a specific page of the documentation you wouldn’t know that.” The “cost” of providing both values up front is negligible to me, but the benefit is high. 

Because we are talking about internationalization, I also think it might be beneficial to discuss the model for resource bundles. While it would be nice if every contributor of a processor provided their own resource bundles, I think this really fragments the operation and will make large-scale translation efforts very difficult. What if, instead, a global resource bundle per language/locale was provided, and individual messages were collected in a single location, and namespaced appropriately? Thus the Kafka processor batch size key translation and ExecuteSQL processor batch size key translation could both be declared in one resource file. 

Something like:

messages_es_ES.properties:

org.apache.nifi.processors.standard.ExecuteSQL.batch_size=tamaño del lote
org.apache.nifi.processors.kafka.PutKafka=tamaño del lote de mensajes

This would make it much easier for someone contributing translation services to focus on a specific language in one place, rather than searching the entire application for 100+ resource bundles for their language alone. I don’t think we can expect someone who contributes specific processor functionality to be able to translate those messages into multiple, much less every, language, but I think it is more reasonable that someone capable of translating Kafka messages to another language can do the same for SQL messages. 





Andy LoPresto
PGP Fingerprint: 70EC B3E5 98A6 5A3F D3C4  BACE 3C6E F65B 2F7D EF69

On May 6, 2016, at 8:04 AM, Brandon DeVries <[hidden email]> wrote:

Joe,

Alright, I buy that.  As a community, we try not to do #2 (stop
laughing...). But we still allow someone writing / maintaining custom
components to do so without figuring out how to set up a resource bundle.
+1.

Brandon

On Fri, May 6, 2016 at 10:51 AM Joe Witt <[hidden email]> wrote:

Brandon

Understood.  I'd say I lean more towards providing 1, 2, and 3 because
allows us to be flexible in what we accept and conservative in what we
do.  By that I mean we as a community can go ahead and enforce that we
basically just adopt 3 and use RTC to help enforce that.  But that
from an API and doing examples perspective we allow 1 and 2 as well.
I don't think it adds much weight but does add flexibility.

Like you, I think I'm ok either way.  This was a good discussion and
ending up with supporting internationalization and flexibility in
displayed name while retaining configuration sanity - is a win.

Thanks
Joe

On Fri, May 6, 2016 at 10:45 AM, Brandon DeVries <[hidden email]> wrote:
I guess my only objection is, do we really need Option 2?  If all you
want
to do is provide a name, fine.  But, if you want to change the name (for
better communicating the purpose to the user, be that in English, French,
or Esperanto) option 3 provides the mechanism for that.  It's not as easy
for the developer as option 2, but it has the benefit of starting the
process of creating resource bundles for different languages (and
potentially deprecating a semi-confusing option).  This means that
someone
wanting to add support for a different language (e.g. french) has a lower
barrier to entry, in that they can just find the
"displayNames.properties"
file, and put a "displayNames_fr_FR.properties next to it.  Ultimately
its
a trade off of conveniences... the developer wanting to change a property
name vs. a potential contributor wanting to improve language support.  I
guess I would argue that this shouldn't be that hard for a developer to
figure out, and lean in favor of encouraging language contributions.

Ultimately, it isn't' that strong an objection though, and it does sound
like this is heading in the right direction...

Brandon

On Fri, May 6, 2016 at 10:25 AM Joe Witt <[hidden email]> wrote:

Definitely on board with the idea that the 'name' will be the key to a
resource bundle.  It does imply such names will need to follow
necessary conventions to be valid resource bundle keys.

However, in the spirit of always thinking about the developer path to
productivity I am hopeful we can come up with a nice way to not
require them to setup a resource bundle.

The idea being that the following order of operations/thought would
exist:

1) How can I provide a new property to this processor?
Answer: Add a property descriptor and set the name.  This name will be
used to refer to the property descriptor whenever serialized/saving
the config and it will be rendered through the REST API and thus made
available as the property name in the UI.

2) Oh snap.  I wish I had used a different name because I've found a
better way to communicate intent to the user.  How do I do this?
Answer: Go ahead and set displayName.  NiFi will continue to use the
'name' for serialization/config saving but will use the displayName
for what is shown to the user in the UI.

3) I would like to support locale sensitive representations of my
property name.  How can I do this?
Answer: Add a resource bundle with entries for your property 'name'
value.  This means the resource bundle needs to exist and your
property 'name' must adhere to resource bundle key naming requirements
[1].  If this is supplied and can be looked up then this will be used
and otherwise will fallback to using displayName value if present and
otherwise will fallback to using the value of 'name'.

And in any event we still need to better document/articulate this
model as the root of this thread was that we hadn't effectively
communicated the existence of displayName.  I agree this discussion
ended up getting us to a great place though as we should all strive to
support internationalization.

With an approach like this I am onboard.  I think this balances our
goals of having a simple to use API but also allows those who want to
support multiple locales to do so cleanly.

Thanks
Joe

[1]
https://docs.oracle.com/javase/tutorial/i18n/resbundle/propfile.html

On Fri, May 6, 2016 at 9:33 AM, Brandon DeVries <[hidden email]> wrote:
+1.  I like that better.  Deprecate displayName(), and set it
"automatically" based on the locale from properties.  The name of the
property (which should never change) is the key into the
ResourceBundle.

Brandon


On Fri, May 6, 2016 at 9:24 AM Matt Burgess <[hidden email]>
wrote:

Same here. Internationalization is often implemented as properties
files/resources, where you possibly load in a file based on the
system
setting for Locale (like processor_names_en_US.properties). If we
were
to do internationalization this way (i.e. a non-code based solution,
which is more flexible), then ironically displayName() might/should
be
deprecated in favor of using the value of name() as the key in a
properties/lookup file; the corresponding value would be the
appropriate locale-specific "display name".

Brandon's links show this approach, I have seen this i18n approach on
other projects/products and it seems to work pretty well.

Regards,
Matt

On Fri, May 6, 2016 at 9:11 AM, Joe Witt <[hidden email]> wrote:
I share Bryan's perspective.

On Fri, May 6, 2016 at 9:05 AM, Bryan Bende <[hidden email]>
wrote:
I might just be resistant to change, but I am still on the fence a
little
bit...

In the past the idea has always been you start out with name, and
if
you
later need to change what is displayed in the UI, then you add
displayName
after the fact.

It sounds like the issue is that a lot of people don't know about
displayName, so I am totally in favor of increasing awareness
through
documentation,
but I'm struggling with telling people that they should set
displayName
as
the default behavior.

For code that is contributed to NiFi, I would expect the
PMC/committer
doing the review/merging to notice if an existing property name
was
being
changed and point that out in the review.
If it was someone else's custom NAR, or even made it through the
review, I
think what happens is that the flow starts up and the
processor/component
becomes invalid because it now has an unknown property.
This is the same behavior when we remove a property from an
existing
processor and someone upgrades, and we deemed this acceptable
behavior
for
that scenario.

The internationalization aspect could possibly sell me more, but I
think I
would need someone to explain how internationalization would be
implemented, and how setting the displayName helps.
What Brandon described makes sense to me because it is something
outside
the Java code, which is different then saying all
PropertyDescriptor
instances need name and displayName.

-Bryan


On Fri, May 6, 2016 at 8:44 AM, Brandon DeVries <[hidden email]>
wrote:

+1.  I think being able to move the displayName out of code an
into
config
/ ResourceBundle will make it much easier to support i18n[1].
If no
config
is provided, the name is the default... otherwise, the name
displayed
is
determined by the locale.  Updating the mock framework to
complain
about
the absence of a ResourceBundle will encourage adoption, and
we'll
gradually work our way to not being English only.

Brandon

[1]
https://docs.oracle.com/javase/tutorial/i18n/intro/quick.html

On Thu, May 5, 2016 at 11:46 PM Adam Lamar <[hidden email]

wrote:

On Tue, May 3, 2016 at 12:09 PM, Andy LoPresto <
[hidden email]

wrote:

As a result of some conversations and some varying feedback
on
PRs, I’d
like
to discuss with the community an issue I see with
PropertyDescriptor
name
and displayName attributes. I’ll describe the scenarios that
cause
issues
and my proposed solution, and then solicit responses and
other
perspectives.

Andy,

I'd be +1 on this as well. I think the power of this approach
will
become more clear over time, and some of the automation will
make
it
possible to more widely enforce.

What do you think about a mixed mode where config reading code
can
fetch the property value with either the name or display name
as
the
key, defaulting to the name if it is present? A sample read of
flow.xml.gz might look like this:

* Processor asks for value of MY_CONFIG_PROPERTY
* Configuration code looks for key "my-config-property",
returns
if
present
* Configuration code looks for key "My Config Property",
returns
if
present
* On finding no valid key, configuration returns
blank/default/null
value (whatever is done today)

On configuration write, the new attributes could be written as
the
normalized name (instead of display name), to allow processors
that
have made the switch to start using the normalized name field
and
start taking advantage of the new features around it (e.g.
internationalization). Perhaps a disadvantage to this approach
is
that
it auto-upgrades an existing flow.xml.gz, making it difficult
to
downgrade from (for example) 0.7 back to 0.6.

A strategy like this (or something similar) might help speed
adoption
by making the process a bit smoother for existing flow.xml.gz
files
and easier to upgrade processors incrementally. At 1.0, display
names
as configuration keys could be dropped, and the upgrade path
for
users
on old 0.x releases would be to upgrade to the latest 0.x
before
making the jump to 1.0.

Cheers,
Adam







signature.asc (859 bytes) Download Attachment
12