ChrisSamo632 commented on pull request #4822:
URL: https://github.com/apache/nifi/pull/4822#issuecomment-808751275


   @turcsanyip started to have a look at making these changes. Using KCL 1.13.3 
instead of 1.14.x appears to be straight forward (i.e. no big API changes).
   
   The refactoring to use a single Worker per processor and yield once the 
Worker has been setup looks fairly straight forward and I agree a simpler 
implementation *except* that if I am to follow the `ConsumeAzureEventHub` 
approach and use the `ProcessSessionFactory` version of `onTrigger`, I have to 
extend the `AbstractSessionFactoryProcessor` base class, but the existing AWS 
processors have a fairly long chain that extends from `AbstractProcessor` - I 
don't want to unmarry this processor from all the existing AWS processors and 
moving all AWS processors to use the different base class feels like a big 
change and not something we'd want to do.
   
   Is there a straight forward way around this do you think? OIne concern I had 
with the original implementation was the fact that I was holding on to a single 
`ProcessSession` and comitting it multiple times (i.e. after every set of 
messages had been processed by the KCL RecordProcessor) - using the 
ProcessSessionFactory approach to create a new session every time a new set of 
Kinesis messages are received would seem better... aside for the above issue of 
the different abstract processors.
   
   I'll look again at this further, but any guidance is welcome!
   
   
   @pvillard31 having looked again at the Record processing now I've the Azure 
processor to compare with, I think includiung them here should be fairly 
straight forward (famous last words).


-- 
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]


Reply via email to