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 <b...@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 <adamond...@gmail.com> wrote: > > > On Tue, May 3, 2016 at 12:09 PM, Andy LoPresto <alopre...@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 > > >
