ChrisSamo632 edited a comment 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)
   
   
   EDIT: the Session Factory problem can be sorted by changing the base 
abstract AWS processor to extend `AbstractSessionFactoryProcessor` and copying 
the `AbstractProcessor` methods into there (but not making the overridden 
`onTrigger` method as `final`) - this retains the same functionality for most 
AWS processors but allows the `ConsumeKinesisStream` processor access to the 
`ProcessSessionFactory`. An akhenaten would be to remove the final modifier 
from the `AbstractProcessor` `onTrigger` that accepts the factory (but that 
impacts nearly all processors and the method is presumably `final` for good 
reason)


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