msharee9 edited a comment on issue #613: Minificpp 927 Nanofi tailfile 
delimited processor
URL: https://github.com/apache/nifi-minifi-cpp/pull/613#issuecomment-520621919
 
 
   > It's getting better, I appreciate that it can now transmit flow files to 
NiFi.
   > 
   > I have still have some concerns to address:
   > -The mixed mode seems to introduce a lot of complexity but not sure it 
worth the effort, do you really think we need it? @phrocker what do you think?
   > -There is no logging even though Nanofi supports it. Could you take a look 
at site2site and logging part of nanofi for some inspiration?
   > -Given the number of allocations I'm a bit afraid of introducing leaks, 
could you try running valgrind or any other dynamic analytics tool that may 
help us reduce this risk?
   
   I want to keep the mixed mode. There is no harm here I guess. It is not as 
complicated to use or to understand from a users perspective. It is an 
additional tool in hand if chunk and delimited do not suffice the needs of the 
user.
   I had also updated code to rectify valgrind leaks. There are some leaks 
already present in the code mostly due to the use of C++ code from minifi 
mostly due to cyclic references I believe. I took care of any additional leaks 
injected by the nanofi tailfile implementation. I do not want to deal with 
those already present leaks because we will anyhow move away from the use of 
minifi code.
   I will look into logging once we start the EFM ECU integration 
implementation. But for now I think this PR is pretty good to be merged.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to