Skip to content

Only issue MESSAGE_WARN_NO_REQUIREMENTS if there are no containers #1558

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Only issue MESSAGE_WARN_NO_REQUIREMENTS if there are no containers #1558

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion planemo/linters/biocontainer_registered.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
def lint_biocontainer_registered(tool_source, lint_ctx):
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()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should still check that we have a biocontainer here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kostrykin can you try to change

planemo/planemo/lint.py

Line 49 in ac2191d

linters.extend(["version_bumped"])
to linters.extend(["version_bumped", "requirements_in_conda", "biocontainer_registered"]).

Then it might be possible to really add these linters to the skip list.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should also be able to just omit the --biocontainers flag

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, but if you want to use the linter for other tools it could be handy.

if not containers:
lint_ctx.warn(MESSAGE_WARN_NO_REQUIREMENTS)
return

mulled_targets = [build_target(c.package, c.version) for c in conda_targets]
Expand Down
Loading