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 <b...@jhu.edu> 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 < > ozhurakou...@hortonworks.com> 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 <joe.w...@gmail.com> 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 <b...@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 <mattyb...@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.w...@gmail.com> wrote: >> >>>> I share Bryan's perspective. >> >>>> >> >>>> On Fri, May 6, 2016 at 9:05 AM, Bryan Bende <bbe...@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 <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 >> >>>>>>> >> >>>>>> >> >>> >> > >> >>
