nifi-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joe Witt <joe.w...@gmail.com>
Subject Re: [discuss] PropertyDescriptor name and displayName attributes
Date Fri, 06 May 2016 14:25:06 GMT
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 <brd@jhu.edu> 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 <mattyb149@gmail.com> 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 <joe.witt@gmail.com> wrote:
>> > I share Bryan's perspective.
>> >
>> > On Fri, May 6, 2016 at 9:05 AM, Bryan Bende <bbende@gmail.com> 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 <brd@jhu.edu> 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 <adamonduty@gmail.com>
>> wrote:
>> >>>
>> >>> > On Tue, May 3, 2016 at 12:09 PM, Andy LoPresto <alopresto@apache.org
>> >
>> >>> > 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
>> >>> >
>> >>>
>>

Mime
View raw message