Repository navigation
Fix plugin functions that return or take a channel - #7715
Conversation
A plugin method annotated with @function that returns a DataflowWriteChannel was picked up by the legacy factory detection and registered as a channel factory, so it could not be called as a plain function. Skip @function methods in the factory fallback so they are registered as functions. Signed-off-by: Ben Sherman <bentshermann@gmail.com>
✅ Deploy Preview for nextflow-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
jorgee
left a comment
There was a problem hiding this comment.
LGTM. The guard fixes the issue, and the tests cover it. They fail without the fix.
Some non-blocking comments. They are outside the scope of this PR but related:
1. Operator fallback has the same problem
getDeclaredOperatorExtensionMethods0 registers any public method whose first parameter is a DataflowReadChannel as an operator, even if it has @Function. So an operator rewritten as a plain function still breaks:
@Function
DataflowWriteChannel dedupe(DataflowReadChannel source) { ... }dedupe(ch) returns an OpCall instead of the channel, and the log says it should be marked @Operator. The same guard would fix it:
if( handle.isAnnotationPresent(Function) ) continue2. @Function together with @Factory or @Operator
If a method has both, the factory or operator wins, and the function can't be called. There is no warning. It could fail fast with a clear error.
3. Docs
It would help to document the pattern in developing-plugins.mdx: a channel source as a @Function that returns a channel, called directly, which also works in typed scripts.
Skip @function methods in the legacy operator detection, so that a function which takes a channel is registered as a function instead of an operator. Fail when a method is annotated with @function and @factory or @operator. Document how to define channel factories and operators as functions. Signed-off-by: Ben Sherman <bentshermann@gmail.com>
Align the test plugin functions with the documented signatures, test the documented pipeline in typed and untyped scripts, and test that plugin factories and operators are unsupported in typed scripts. Simplify the docs for factories and operators as functions. Signed-off-by: Ben Sherman <bentshermann@gmail.com>
|
Thanks Jorge for the review.
I tested this and confirmed that operators can also be written as plain functions with the same override fix. They just need to take a DataflowWriteChannel instead of a DataflowReadChannel.
Added an error for this.
Updated the docs to show how to rewrite factories / operators as plain functions. Follow-up: consider backporting to 26.04 to enable this pattern, since this is essentially a bug fix. |
Related to #7694
A plugin method annotated with
@Functioncan't be called as a function if it returns or takes a channel. It fails withMissing process or function <name>(), in both typed and untyped scripts.The legacy fallbacks in
PluginExtensionProviderregister any public method that returns a write channel as a factory, and any method whose first parameter is a read channel as an operator.loadPluginExtensionMethodschecks factories and operators before functions, so the@Functionannotation is ignored.Changes:
@Functionmethods in the factory and operator fallbacks, so they are registered as functions.@Functionand@Factoryor@Operator. Previously the factory or operator annotation won and nothing was reported.developing-plugins.mdx): replace the tip at the end of the Operators section with a section on writing channel factories and operators as functions, noting that plugin factories and operators aren't supported with static typing.With this change, a plugin can provide channel sources and operators as plain functions, e.g.
fromQuery(...)andsqlInsert(rows, ...). These also work in typed scripts:WorkflowBindingwraps the result in aChannelImpland unwraps channel arguments to v1.Note on operator functions: channel arguments arrive as a
DataflowBroadcast, which is not aDataflowReadChannel. The parameter should therefore be aDataflowWriteChannel, converted withCH.getReadChannel(), as the docs describe.Tests:
HelloExtensionfixture:reverseFn(a factory function) andgoodbyeFn(an operator function).PluginExtensionMethodsTest: both functions in untyped and typed scripts, including theChannelImplwrapping and typed operators applied to the result.PluginExtensionProviderTest: detection of channel functions, and rejection of conflicting annotations.nf-commonsandnextflowpass (203).Compatibility: a plugin that annotates a channel method with
@Functionbut calls it aschannel.x(...)orch.x(...), or that puts@Functiontogether with@Factoryor@Operator, would no longer work. The first pattern only worked through the legacy fallback.