Skip to content
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.

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

Add custom registry to nfcore modules lint #2313

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
- Fixed the Slack report to include the pipeline name ([#2291](https://github.com/nf-core/tools/pull/2291))
- Fix link in the MultiQC report to point to exact version of output docs ([#2298](https://github.com/nf-core/tools/pull/2298))
- Fix parsing of container directive when it is not typical nf-core format ([#2306](https://github.com/nf-core/tools/pull/2306))
- Add ability to specify custom registry for linting modules, defaults to quay.io ([#2313](https://github.com/nf-core/tools/pull/2313))

### Download

Expand Down
6 changes: 5 additions & 1 deletion nf_core/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,9 @@ def create_test_yml(ctx, tool, run_tests, output, force, no_prompts):
@click.pass_context
@click.argument("tool", type=str, required=False, metavar="<tool> or <tool/subtool>")
@click.option("-d", "--dir", type=click.Path(exists=True), default=".", metavar="<pipeline/modules directory>")
@click.option(
"-r", "--registry", type=str, metavar="<registry>", default="quay.io", help="Registry to use for containers"
)
@click.option("-k", "--key", type=str, metavar="<test>", multiple=True, help="Run only these lint tests")
@click.option("-a", "--all", is_flag=True, help="Run on all modules")
@click.option("-w", "--fail-warned", is_flag=True, help="Convert warn tests to failures")
Expand All @@ -821,7 +824,7 @@ def create_test_yml(ctx, tool, run_tests, output, force, no_prompts):
)
@click.option("--fix-version", is_flag=True, help="Fix the module version if a newer version is available")
def lint(
ctx, tool, dir, key, all, fail_warned, local, passed, sort_by, fix_version
ctx, tool, dir, registry, key, all, fail_warned, local, passed, sort_by, fix_version
): # pylint: disable=redefined-outer-name
"""
Lint one or more modules in a directory.
Expand All @@ -846,6 +849,7 @@ def lint(
)
module_lint.lint(
module=tool,
registry=registry,
key=key,
all_modules=all,
print_results=True,
Expand Down
16 changes: 9 additions & 7 deletions nf_core/modules/lint/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ def get_all_lint_tests(is_pipeline):
def lint(
self,
module=None,
registry="quay.io",
key=(),
all_modules=False,
print_results=True,
Expand Down Expand Up @@ -227,11 +228,11 @@ def lint(

# Lint local modules
if local and len(local_modules) > 0:
self.lint_modules(local_modules, local=True, fix_version=fix_version)
self.lint_modules(local_modules, registry=registry, local=True, fix_version=fix_version)

# Lint nf-core modules
if len(remote_modules) > 0:
self.lint_modules(remote_modules, local=False, fix_version=fix_version)
self.lint_modules(remote_modules, registry=registry, local=False, fix_version=fix_version)

if print_results:
self._print_results(show_passed=show_passed, sort_by=sort_by)
Expand Down Expand Up @@ -264,12 +265,13 @@ def filter_tests_by_key(self, key):
# If -k supplied, only run these tests
self.lint_tests = [k for k in self.lint_tests if k in key]

def lint_modules(self, modules, local=False, fix_version=False):
def lint_modules(self, modules, registry="quay.io", local=False, fix_version=False):
"""
Lint a list of modules

Args:
modules ([NFCoreModule]): A list of module objects
registry (str): The container registry to use. Should be quay.io in most situations.
local (boolean): Whether the list consist of local or nf-core modules
fix_version (boolean): Fix the module version if a newer version is available
"""
Expand All @@ -290,9 +292,9 @@ def lint_modules(self, modules, local=False, fix_version=False):

for mod in modules:
progress_bar.update(lint_progress, advance=1, test_name=mod.module_name)
self.lint_module(mod, progress_bar, local=local, fix_version=fix_version)
self.lint_module(mod, progress_bar, registry=registry, local=local, fix_version=fix_version)

def lint_module(self, mod, progress_bar, local=False, fix_version=False):
def lint_module(self, mod, progress_bar, registry, local=False, fix_version=False):
"""
Perform linting on one module

Expand All @@ -311,7 +313,7 @@ def lint_module(self, mod, progress_bar, local=False, fix_version=False):

# Only check the main script in case of a local module
if local:
self.main_nf(mod, fix_version, progress_bar)
self.main_nf(mod, fix_version, registry, progress_bar)
self.passed += [LintResult(mod, *m) for m in mod.passed]
warned = [LintResult(mod, *m) for m in (mod.warned + mod.failed)]
if not self.fail_warned:
Expand All @@ -323,7 +325,7 @@ def lint_module(self, mod, progress_bar, local=False, fix_version=False):
else:
for test_name in self.lint_tests:
if test_name == "main_nf":
getattr(self, test_name)(mod, fix_version, progress_bar)
getattr(self, test_name)(mod, fix_version, registry, progress_bar)
else:
getattr(self, test_name)(mod)

Expand Down
24 changes: 16 additions & 8 deletions nf_core/modules/lint/main_nf.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
log = logging.getLogger(__name__)


def main_nf(module_lint_object, module, fix_version, progress_bar):
def main_nf(module_lint_object, module, fix_version, registry, progress_bar):
"""
Lint a ``main.nf`` module file

Expand Down Expand Up @@ -121,7 +121,7 @@ def main_nf(module_lint_object, module, fix_version, progress_bar):
module.passed.append(("main_nf_script_outputs", "Process 'output' block found", module.main_nf))

# Check the process definitions
if check_process_section(module, process_lines, fix_version, progress_bar):
if check_process_section(module, process_lines, registry, fix_version, progress_bar):
module.passed.append(("main_nf_container", "Container versions match", module.main_nf))
else:
module.warned.append(("main_nf_container", "Container versions do not match", module.main_nf))
Expand Down Expand Up @@ -209,12 +209,20 @@ def check_when_section(self, lines):
self.passed.append(("when_condition", "when: condition is unchanged", self.main_nf))


def check_process_section(self, lines, fix_version, progress_bar):
"""
Lint the section of a module between the process definition
def check_process_section(self, lines, registry, fix_version, progress_bar):
"""Lint the section of a module between the process definition
and the 'input:' definition
Specifically checks for correct software versions
and containers

Args:
lines (List[str]): Content of process.
registry (str): Base Docker registry for containers. Typically quay.io.
fix_version (bool): Fix software version
progress_bar (ProgressBar): Progress bar to update.

Returns:
Optional[bool]: True if singularity and docker containers match, False otherwise. If process definition does not exist, None.
"""
# Check that we have a process section
if len(lines) == 0:
Expand Down Expand Up @@ -288,7 +296,7 @@ def check_process_section(self, lines, fix_version, progress_bar):
else:
self.failed.append(("docker_tag", "Unable to parse docker tag", self.main_nf))
docker_tag = None
if l.startswith("quay.io/"):
if l.startswith(registry):
l_stripped = re.sub(r"\W+$", "", l)
self.failed.append(
(
Expand All @@ -303,7 +311,7 @@ def check_process_section(self, lines, fix_version, progress_bar):
# Guess if container name is simple one (e.g. nfcore/ubuntu:20.04)
# If so, add quay.io as default container prefix
if l.count("/") == 1 and l.count(":") == 1:
l = "quay.io/" + l
l = "/".join([registry, l]).replace("//", "/")
url = urlparse(l.split("'")[0])

# lint double quotes
Expand All @@ -326,7 +334,7 @@ def check_process_section(self, lines, fix_version, progress_bar):
)

# lint more than one container in the same line
if ("https://containers" in l or "https://depot" in l) and ("biocontainers/" in l or "quay.io/" in l):
if ("https://containers" in l or "https://depot" in l) and ("biocontainers/" in l or l.startswith(registry)):
self.warned.append(
(
"container_links",
Expand Down
10 changes: 10 additions & 0 deletions tests/modules/lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,16 @@ def test_modules_lint_multiple_remotes(self):
assert len(module_lint.warned) >= 0


def test_modules_lint_registry(self):
mashehu marked this conversation as resolved.
Show resolved Hide resolved
"""Test linting the samtools module and alternative registry"""
self.mods_install.install("samtools")
module_lint = nf_core.modules.ModuleLint(dir=self.pipeline_dir)
module_lint.lint(print_results=False, registry="public.ecr.aws", module="samtools")
assert len(module_lint.failed) == 0, f"Linting failed with {[x.__dict__ for x in module_lint.failed]}"
assert len(module_lint.passed) > 0
assert len(module_lint.warned) >= 0


def test_modules_lint_patched_modules(self):
"""
Test creating a patch file and applying it to a new version of the the files
Expand Down