knime-ip / knip-scijava

KNIP - SciJava Commands Plugin
2 stars 3 forks source link

StatusService: Wrap ExecutionContext in StatusService #14

Open dietzc opened 8 years ago

dietzc commented 8 years ago

Uses and enhances: https://github.com/scijava/scijava-common/blob/master/src/main/java/org/scijava/app/StatusService.java

This can directly be used in the scripting.

Squareys commented 7 years ago

@dietzc This would bring back the requirement for a local Service. Still do?

dietzc commented 7 years ago

I'm not sure what exactly we want here. We have to think about it:

Feel free to add more. Concerning the local Context problem. Yes, I think then we have to introduce a Context per run() in a Node.

Squareys commented 7 years ago

Cancel Command if Cancelable.

Referring to calling cancel() on the Command when the KNIME node is cancelled, in case it implements Cancelable, else just shoot it down cold blood, correct?

Log status messages from Commands somewhere. Maybe a bit related to some of the discussion here: http://forum.imagej.net/t/logging-in-fiji-and-imagej/2664/10

KNIMELogService and -StatusService, manually instantiated and manually resolved by NodeModule is my solution, since they need to hold NodeLogger/ExecutionContext to delegate the status messages too. Otherwise we would need a local context, since these are Node-local states.

Commands should be able to set sub-progress , in contrast to just determine the Commands progress via the number of images which have been processed divided by the number of total images. Useful if we have Commands which run on only a few images but for a long, long time.

Hurray, that is simple! KNIME ProgressMonitors already provide this functionality! :) We would just pass a subprogressmonitor to the StatusService.

dietzc commented 7 years ago

Referring to calling cancel() on the Command when the KNIME node is cancelled, in case it implements Cancelable, else just shoot it down cold blood, correct?

If it works, then yes, if possible, kill the thread!

KNIMELogService and -StatusService, manually instantiated and manually resolved by NodeModule is my solution, since they need to hold NodeLogger/ExecutionContext to delegate the status messages too. Otherwise we would need a local context, since these are Node-local states.

I think the KNIMELogService can be auto-injected. I don't see why we need a single instance per node?

Hurray, that is simple! KNIME ProgressMonitors already provide this functionality! :) We would just pass a subprogressmonitor to the StatusService.

Juchu!

Squareys commented 7 years ago

I think the KNIMELogService can be auto-injected. I don't see why we need a single instance per node?

Because we would not be able to distinguish which node the log messages come from.

Juchu!

:tada:

dietzc commented 7 years ago

Because we would not be able to distinguish which node the log messages come from.

True, so we have to use the SubContext idea of @gab1one again for LogService, StatusService and ExecutionService/CancellationService. However, this SubContext should be per execute(). So we have to make sure, that the actual Module is also instantiated on execute.

Please check performance of the SubContext creation.