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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> > 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 <[email protected]> wrote: > >> >> > I share Bryan's perspective. > >> >> > > >> >> > On Fri, May 6, 2016 at 9:05 AM, Bryan Bende <[email protected]> > 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 <[email protected]> > 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 <[email protected] > > > >> >> wrote: > >> >> >>> > >> >> >>> > On Tue, May 3, 2016 at 12:09 PM, Andy LoPresto < > >> [email protected] > >> >> > > >> >> >>> > 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 > >> >> >>> > > >> >> >>> > >> >> > >> >
