Add onWorkflowPublish event to TraceObserverV2#6701
Conversation
✅ Deploy Preview for nextflow-docs-staging ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
3a8a2d1 to
b5a155d
Compare
Fire a new event during PublishOp::onNext for each value being published to a workflow output. Unlike onWorkflowOutput which fires after completion, this provides real-time notification with the complete channel value as each emission is published. Signed-off-by: Rob Syme <rob.syme@gmail.com>
b5a155d to
6333b8c
Compare
….groovy Signed-off-by: Rob Syme <rob.syme@gmail.com>
|
I think this is the right idea I'm wondering whether we should only fire this event for queue channels and not value channels. Since a value channel is just a single value, the |
pditommaso
left a comment
There was a problem hiding this comment.
as a developer I wonder what's the difference of onWorkflowOutput vs onWorkflowPublish vs onFilePublish.
At least there should be a very detailed docs and javadoc describing what event is meant for and when to use it
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
|
I updated the java docs to be more comprehensive. I will follow up with a guide in the docs for how to use the trace observer |
|
Amazing. Thanks team ❤️ 🙏 |
Co-authored-by: Ben Sherman <bentshermann@gmail.com>
This reverts commit d4cf0ca.
|
Team, I've reverted this PR from master. This change introduces a breaking change with existing plugins (avoiding which was one of the main reasons observer v2 was introduced). We could, of course, introduce this kind of change, but it would be preferable to keep as a last resort. I want to push further on thinking about how the requested use case can be achieved without introducing a new event handler. (related 06cd260) |
|
Apologies. Is the incompatibility that plugins without the new method no longer adhere to the expanded interface? |
|
No problem! Yeah, It's a kind of Groovy limitation, because Java interfaces default method is designed to avoid that. Apart this, would not your use case fit into |
|
@pditommaso the rationale for this new event is that it allows you to respond to published values more quickly. For example, when publishing a channel of BAM/BAI samples, it allows you to react to each sample as it is published, whereas Can you elaborate on how this change was breaking? If adding a new event with default impl is breaking, then you are implying that we can't add to TraceObserverV2 at all... Also, TraceObserverV2 is annotated with |
|
Since this event was a performance enhancement rather than enabling new functionality, we aren't going to worry about it right now. But I would like to investigate the compatibility issues as I think it should be possible to avoid them. Notes for future investigation:
If these two issues can be resolved, then we should be able to add new trace events without breaking existing plugins. Until then, we will try to add new events only when absolutely necessary. |
Summary
Adds a new
onWorkflowPublishevent toTraceObserverV2that fires for each value as it is published to a workflow output.This addresses the request in #6680 for richer context during file publishing. The new event provides:
name- the workflow output name (e.g., "alignedBams")value- the complete channel value being published (which may contain files alongside metadata like sample IDs)Event timeline
Channel value 1 arrives → onWorkflowPublish ← NEW (fires per value)
→ onFilePublish (for each file)
Channel value 2 arrives → onWorkflowPublish ← NEW
→ onFilePublish (for each file)
...
All values complete → onWorkflowOutput (fires once at end)
Unlike
onWorkflowOutputwhich fires after all values have been published,onWorkflowPublishfires in real-time as each value flows through, givingTest plan
OutputDslTesttests updated to verifynotifyWorkflowPublishis called