Github user markap14 commented on the issue:
https://github.com/apache/nifi/pull/836
@olegz @joewitt I have pushed a new update to this PR. I did it as a
separate commit so that you can see the changes made. I also rebased against
master.
Fixed issue where we were calling @OnUnscheduled methods before invoking
SchedulingAgent.unschedule - that was a good catch. Also addressed the JavaDocs
and ensured that we use the appropriate Nar ClassLoader.
re: callback vs. passing in ScheduleState - I agree that both do have their
pro's & con's. However, when I was reviewing the code I thought that it was
quite difficult to follow, with the callback. The callback was only about 2-3
lines of code, and could easily just be moved into the StandardProcessorNode,
and this made the code much more straight-forward to read and understand.
Additionally, I do not consider it a leaky abstraction, given that this is the
'start' method of ProcessorNode, and this class's job is to handle the
"framework-y aspect of processors."
Should we need to change it to use a callback in the future, we certainly
can do so. If we do, though, I think we should create an interface that has a
specific method whose name makes sense and is well documented rather than using
a Callable<Boolean> to perform the operation & documenting how that Callable
should operate in the JavaDocs for the method called. I think this is what
really made it confusing to understand.
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
