Mark, I am fine including it in the best practices validator linked in your message, but it doesn’t look like there has been any work on that ticket since January 2016.
As a quick solution (and to answer Oleg’s question), I think we could add a simple Checkstyle regex rule [1] to detect the presence of displayName attribute in PropertyDescriptor declarations. We should work to clean up the deprecation warnings that currently print during a build in order to make any messages easier to read and more impactful. [1] http://checkstyle.sourceforge.net/config_regexp.html <http://checkstyle.sourceforge.net/config_regexp.html> Andy LoPresto alopre...@apache.org alopresto.apa...@gmail.com PGP Fingerprint: 70EC B3E5 98A6 5A3F D3C4 BACE 3C6E F65B 2F7D EF69 > On May 3, 2016, at 11:51 AM, Mark Payne <marka...@hotmail.com> wrote: > > 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 >
signature.asc
Description: Message signed with OpenPGP using GPGMail
