Only issue MESSAGE_WARN_NO_REQUIREMENTS if there are no containers
#1558
Conversation
|
This is logged as a warning, which I think is the appropriate level for tools that only run containerized. You can chose to ignore particular warnings IIRC, ping @bernt-matthias. |
Yes, but since weekly linting runs with
I'd very much appreciate getting to know how to ignore this warning! |
Can you try to add Adding a check for defined containers seems also reasonable. |
This gives the error
|
| conda_targets = tool_source_conda_targets(tool_source) | ||
| if not conda_targets: | ||
| lint_ctx.warn(MESSAGE_WARN_NO_REQUIREMENTS) | ||
| _, containers, *_ = tool_source.parse_requirements_and_containers() |
There was a problem hiding this comment.
This should still check that we have a biocontainer here.
There was a problem hiding this comment.
Maybe I'm missing something here. But if this should always check for a Biocontainer, then this PR is pointless (the whole idea of this PR is to only check for Biocontainers in cases where it makes sense to expect a Biocontainer).
Why would it make sense to check for a Biocontainer if the wrapper specifies a custom container image? If I'm not mistaken, this is like producing a warning for every wrapper that uses a custom container image?
Still, if this PR is pointless, I'm still wondering how we are going to suppress the warnings for wrappers that use a custom container image. I think there is no mechanism for this in place yet.
There was a problem hiding this comment.
This is a warning that you can choose to ignore (as opposed to errors). The wider Galaxy ecosystem has agreed to bicontainers and caches them on CVMFS, custom images should make the iuc linter fail. You can select the warnings you want to ignore on a per directory basis. You can also ignore the biocontainers linter on a per tool basis. It is a valuable warning.
There was a problem hiding this comment.
If I'm not mistaken, this is like producing a warning for every wrapper that uses a custom container image?
yes, that is the purpose of this linter.
There was a problem hiding this comment.
@kostrykin can you try to change
Line 49 in ac2191d
linters.extend(["version_bumped", "requirements_in_conda", "biocontainer_registered"]).
Then it might be possible to really add these linters to the skip list.
There was a problem hiding this comment.
You should also be able to just omit the --biocontainers flag
There was a problem hiding this comment.
True, but if you want to use the linter for other tools it could be handy.
Currently, running
planemo shed_lint --tools --ensure_metadata --urls --biocontainerson a tool that defines containerized requirements instead of conda packages yields the warningwhichs makes the linting fail. Examples of this behavior are, but are not limited to:
This generally happens in the weekly linting workflows employed by many repositories, e.g.:
https://github.com/galaxyproject/tools-iuc/actions/runs/16405860532/workflow#L99-L107
However, the fact that there are no package requirements defined in those tools is a decision by design of those tools. It makes no sense to check the availability of Biocontainers for those tools, does it? Hence, I would consider these lint failures as false positives.
To cope with that, I suggest to suppress the warning if the tool only defines containerized requirements.