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 <ozhurakou...@hortonworks.com> > 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 <alopre...@apache.org > <mailto:alopre...@apache.org><mailto:alopre...@apache.org > <mailto:alopre...@apache.org>>> 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 > alopre...@apache.org > <mailto:alopre...@apache.org><mailto:alopre...@apache.org > <mailto:alopre...@apache.org>> > alopresto.apa...@gmail.com > <mailto:alopresto.apa...@gmail.com><mailto:alopresto.apa...@gmail.com > <mailto:alopresto.apa...@gmail.com>> > PGP Fingerprint: 70EC B3E5 98A6 5A3F D3C4 BACE 3C6E F65B 2F7D EF69
