Github user joewitt commented on the pull request:
https://github.com/apache/nifi/pull/354#issuecomment-211708711
@YolandaMDavis @olegz some feedback.
1. The com.bazaar.jolt... dependency does not appear to be accounted for in
the LICENSE/NOTICE updates. Their LICENSE does indicate they would like
reference made to 'Copyright 2013-2014 Bazaarvoice, Inc.' This is found in
https://github.com/bazaarvoice/jolt/blob/1271e1919693e63ce1efdf534dfee0e0579d0b2f/LICENSE
So we should propogate that into the NOTICE file as well. Also we need to
reference JOLT in our top level nifi-assembly/NOTICE as well if it is new and
I'd assume at least for Bazaar it would be.
2. I don't believe I follow the recommendation to change the member
variable 'transform' to volatile or to set it to null on stopped. The variable
appears to be set at the proper lifecycle point in NiFi and thus would not be
reassigned in some non-thread safe way. The only question then is could its
use itself be non Thread safe and that does appear questionable to me. In that
case marking the variable as volatile won't help at all anyway. If indeed it
is not thread safe you will want to consider marking the class as executing
serially or you could have a pool of transforms or you could transform batches
at a time. Lots of ways to tackle it. Of course if that is thread safe
already then you are fine and in any event the volatile should be unnecessary.
Finally, I am not aware of any cause for setting it to null during onStopped or
on unScheduled as if the processor was being removed from the flow then it
would get cleaned up anyway. If there was a need to call a close meth
od or something then yes that is good.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
