nifi-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From joewitt <...@git.apache.org>
Subject [GitHub] nifi pull request: NIFI-361 - Create Processors to mutate JSON dat...
Date Tue, 19 Apr 2016 03:20:55 GMT
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message