Unify nf-lang config scopes with runtime classes#6271
Conversation
✅ Deploy Preview for nextflow-docs-staging canceled.
|
christopher-hakkaart
left a comment
There was a problem hiding this comment.
Docs look good. No comments.
824523f to
8ca91ca
Compare
modules/nextflow/src/main/groovy/nextflow/container/ContainerConfig.groovy
Show resolved
Hide resolved
modules/nextflow/src/main/groovy/nextflow/executor/BashWrapperBuilder.groovy
Outdated
Show resolved
Hide resolved
modules/nf-lineage/src/main/nextflow/lineage/cli/LinCommandImpl.groovy
Outdated
Show resolved
Hide resolved
8ca91ca to
9d5e6b8
Compare
modules/nf-lineage/src/main/nextflow/lineage/cli/LinCommandImpl.groovy
Outdated
Show resolved
Hide resolved
modules/nf-lineage/src/main/nextflow/lineage/LinStoreFactory.groovy
Outdated
Show resolved
Hide resolved
modules/nf-lineage/src/main/nextflow/lineage/LinStoreFactory.groovy
Outdated
Show resolved
Hide resolved
61d6cf6 to
963c695
Compare
This comment was marked as resolved.
This comment was marked as resolved.
963c695 to
d3b1149
Compare
This comment was marked as resolved.
This comment was marked as resolved.
|
Tests are passing now and I added a page to the developer docs I also added the JSON and Markdown renderers. They aren't used by Nextflow itself. The JSON renderer is used by the language server, but it will also be needed by plugins in the future so it makes sense to keep the core logic in Nextflow. |
|
Wonder why the config classes need to be specified in the plugin extension files? |
|
Because they extend |
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
57f5693 to
f51d3e2
Compare
|
Got it. Could not make sense to use classic Java service loader to avoid to maintain extension.idx files? |
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
|
I'm fine with using the ServiceLoader if it's something you want to apply to the rest of the codebase, otherwise I would rather stick to the existing system. Maybe the ServiceLoader approach should be investigated separately |
|
My rationale was more that config scope are not strictly plugins, however they may be part of a plugin. Therefore make sense to keep it as it is. Think several tests are still failing |
|
The CI failures are from the plugin registry |
|
let's disable that test(s) temporary |
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
This PR moves the config schema annotations into the runtime so that we don't have to update config options in both places.
Plugin config scopes are now defined in the plugins themselves. Core plugins are typically only loaded when they are actually used (i.e. the awsbatch executor), so when Nextflow validates config at runtime, it will skip validation of core plugin config if the core plugin isn't loaded. This allows the
lintcommand to work the same as before.I will update the language server to extract this metadata at build-time with a custom Gradle task. For third-party plugins we will need to publish this metadata to the plugin registry, but that can be done later.
I have removed several config options that were undocumented or appeared to be legacy options. We can preserve them if needed. See my comments below.
TODO: