onRemoved and onAdded methods signature and behaviour

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

onRemoved and onAdded methods signature and behaviour

Toivo Adams
Hi,

Nifi developer guide:
@OnRemoved
The @OnRemoved annotation causes a method to be invoked before a component is removed from the flow. This allows resources to be cleaned up before removing a component. Methods with this annotation must take zero arguments. If a method with this annotation throws an Exception, the component will still be removed.

But /nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/GetJMSTopic.java

    @OnRemoved
    public void onRemoved(final ProcessContext context) throws IOException, JMSException {
        // unsubscribe from the old subscription.
        unsubscribe(context);
    }

Can onRemoved and onAdded methods take arguments?
Actually I think  ProcessContext context is useful onRemoved and onAdded.

And org.apache.nifi.processor.annotation.OnRemoved  states
 * If any method annotated with this annotation throws, the processor will not
be removed from the graph.

But guide says 'throws an Exception, the component will still be removed.'

toivo
Reply | Threaded
Open this post in threaded view
|

Re: onRemoved and onAdded methods signature and behaviour

Toivo Adams
When using TestRunner I get error

java.lang.AssertionError: Could not invoke methods annotated with @OnAdded annotation due to: java.lang.IllegalArgumentException: Unable to invoke method onAdded on BirtReport[id=fc81e2dd-0b1c-432c-bd34-8e45f6e6494b] because method expects 1 parameters but only 0 were given
        at org.junit.Assert.fail(Assert.java:88)
        at org.apache.nifi.util.StandardProcessorTestRunner.<init>(StandardProcessorTestRunner.java:91)
        at org.apache.nifi.util.TestRunners.newTestRunner(TestRunners.java:24)


Method in BirtReport

    @OnAdded
    public void onAdded(final ProcessContext context) {
        String logDir = context.getProperty(LOG_FILES_DIR).getValue();
        engine = createEngine(logDir);
    }
   

Something is messed up.
It seems onAdded expects no arguments, but error message is wrong.

When I change onAdded to

    @OnAdded
    public void onAdded() {
//    public void onAdded(final ProcessContext context) {
//        String logDir = context.getProperty(LOG_FILES_DIR).getValue();
//        engine = createEngine(logDir);
    }
   
onAdded will be invoked successfully.

toivo
Reply | Threaded
Open this post in threaded view
|

Re: onRemoved and onAdded methods signature and behaviour

Mark Payne
In reply to this post by Toivo Adams

Toivo,




Wow! Sorry, I really messed up that documentation! Let me try to clarify:




@OnRemoved: Methods with this annotation can take a ProcessContext (but do not have to).




@OnAdded: Methods with this annotation do NOT take a ProcessContext. This was done because when the Processor is added, all properties, etc. are the default values, so I wasn’t sure that it needed a ProcessContext. Maybe this should be changed, just for completeness sake so that all of the lifecycle annotations have the ProcessContext.




Whether or not @OnRemoved throwing Exception will prevent a Processor from being removed: At this point, in 0.0.1, if a processor throws an exception at this point, it will not be removed from the graph. But this has caused way too many problems in practice, so in 0.1.0 it is being changed such that this will NOT prevent the Processor from being removed.




The big change in 0.1.0 (that warrants a minor version change instead of an incremental version change) is that Controller Services and Reporting Tasks will be configured via the UI and be more first-class citizens. This means that they need better lifecycle support, and as a result, this is a good opportunity to reconsider how all of the lifecycle annotations work.




The following tickets are related to this:

https://issues.apache.org/jira/browse/NIFI-6

https://issues.apache.org/jira/browse/NIFI-4

https://issues.apache.org/jira/browse/NIFI-251

https://issues.apache.org/jira/browse/NIFI-278   -> This one specifically you will care about 😊

https://issues.apache.org/jira/browse/NIFI-284







Finally, regarding that error message… the intent of the message is to say that the method that you implemented expects 1 argument, but the framework provided 0 arguments (because @OnAdded currently doesn’t allow for a ProcessContext). This, again should be addressed as part of NIFI-278.




I also just created https://issues.apache.org/jira/browse/NIFI-354 to fix the documentation for this.


I apologize for all of confusion - and thank you for bringing this up instead of being annoyed and moving on! 😊


If anything is still unclear, please let me know.


Thanks

-Mark












From: Toivo Adams
Sent: ‎Saturday‎, ‎February‎ ‎14‎, ‎2015 ‎8‎:‎00‎ ‎AM
To: [hidden email]





When using TestRunner I get error

java.lang.AssertionError: Could not invoke methods annotated with @OnAdded
annotation due to: java.lang.IllegalArgumentException: Unable to invoke
method onAdded on BirtReport[id=fc81e2dd-0b1c-432c-bd34-8e45f6e6494b]
because method expects 1 parameters but only 0 were given
        at org.junit.Assert.fail(Assert.java:88)
        at
org.apache.nifi.util.StandardProcessorTestRunner.<init>(StandardProcessorTestRunner.java:91)
        at org.apache.nifi.util.TestRunners.newTestRunner(TestRunners.java:24)


Method in BirtReport

    @OnAdded
    public void onAdded(final ProcessContext context) {
        String logDir = context.getProperty(LOG_FILES_DIR).getValue();
        engine = createEngine(logDir);
    }
   

Something is messed up.
It seems onAdded expects no arguments, but error message is wrong.

When I change onAdded to

    @OnAdded
    public void onAdded() {
//    public void onAdded(final ProcessContext context) {
//        String logDir = context.getProperty(LOG_FILES_DIR).getValue();
//        engine = createEngine(logDir);
    }
   
onAdded will be invoked successfully.

toivo



--
View this message in context: http://apache-nifi-incubating-developer-list.39713.n7.nabble.com/onRemoved-and-onAdded-methods-signature-and-behaviour-tp683p684.html
Sent from the Apache NiFi (incubating) Developer List mailing list archive at Nabble.com.
Reply | Threaded
Open this post in threaded view
|

Re: onRemoved and onAdded methods signature and behaviour

Toivo Adams
Thank you for quick clarification.
Everything will happen, but you are very dedicated.
Keep going.

It seems I can use @OnScheduled and @OnUnscheduled for now.

    @OnScheduled
    public void createEngine(final ProcessContext context) {
    String logDir = context.getProperty(LOG_FILES_DIR).getValue();
    engine = createEngine(logDir);
    }
   
    @OnUnscheduled
    public void destroyEngine(final ProcessContext context) {
    engine.destroy();
    Platform.shutdown();
    engine = null;
    }

Birt engine is thread safe, so I want to reuse it.

Birt itself is more headache currently.
Engine swallows errors and don't throw Exceptions.
So failed reports end up in REL_SUCCESS and not in REL_FAILURE.
Not good.

Toivo
Reply | Threaded
Open this post in threaded view
|

Re: onRemoved and onAdded methods signature and behaviour

Mark Payne
Toivo,

Excellent. OnScheduled and OnUnscheduled are the right place for this. OnAdded and OnRemoved are only called when you drag the processor to the graph or delete it. I.e., not when you restart NiFi.

Thanks
-Mark

Sent from my iPhone

> On Feb 14, 2015, at 9:22 AM, Toivo Adams <[hidden email]> wrote:
>
> Thank you for quick clarification.
> Everything will happen, but you are very dedicated.
> Keep going.
>
> It seems I can use @OnScheduled and @OnUnscheduled for now.
>
>    @OnScheduled
>    public void createEngine(final ProcessContext context) {
>        String logDir = context.getProperty(LOG_FILES_DIR).getValue();
>        engine = createEngine(logDir);
>    }
>
>    @OnUnscheduled
>    public void destroyEngine(final ProcessContext context) {
>        engine.destroy();
>        Platform.shutdown();
>        engine = null;
>    }
>
> Birt engine is thread safe, so I want to reuse it.
>
> Birt itself is more headache currently.
> Engine swallows errors and don't throw Exceptions.
> So failed reports end up in REL_SUCCESS and not in REL_FAILURE.
> Not good.
>
> Toivo
>
>
>
>
> --
> View this message in context: http://apache-nifi-incubating-developer-list.39713.n7.nabble.com/onRemoved-and-onAdded-methods-signature-and-behaviour-tp683p686.html
> Sent from the Apache NiFi (incubating) Developer List mailing list archive at Nabble.com.