From 6a2adc15c72c500fa3297a1553d633d2bffc3eb6 Mon Sep 17 00:00:00 2001 From: Adam Talbot Date: Tue, 13 Jun 2023 16:42:06 +0100 Subject: [PATCH 1/5] feat (2311): Infer docker.registry value from nextflow.config Fixes #2311 --- CHANGELOG.md | 1 + nf_core/__main__.py | 9 +++++++-- nf_core/modules/lint/__init__.py | 27 ++++++++++++++++++--------- nf_core/utils.py | 9 +++++++-- 4 files changed, 33 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index be0ab9828e..d95e84f0a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ - 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)) +- Add ability to interpret `docker.registry` from `nextflow.config` file. If not found defaults to quay.io. ### Download diff --git a/nf_core/__main__.py b/nf_core/__main__.py index 00fba36315..f531108b1b 100644 --- a/nf_core/__main__.py +++ b/nf_core/__main__.py @@ -808,7 +808,12 @@ def create_test_yml(ctx, tool, run_tests, output, force, no_prompts): @click.argument("tool", type=str, required=False, metavar=" or ") @click.option("-d", "--dir", type=click.Path(exists=True), default=".", metavar="") @click.option( - "-r", "--registry", type=str, metavar="", default="quay.io", help="Registry to use for containers" + "-r", + "--registry", + type=str, + metavar="", + default=None, + help="Registry to use for containers. If not specified will interpret it from nextflow.config file at attribute docker.registry", ) @click.option("-k", "--key", type=str, metavar="", multiple=True, help="Run only these lint tests") @click.option("-a", "--all", is_flag=True, help="Run on all modules") @@ -842,6 +847,7 @@ def lint( module_lint = ModuleLint( dir, fail_warned=fail_warned, + registry=ctx.params["registry"], remote_url=ctx.obj["modules_repo_url"], branch=ctx.obj["modules_repo_branch"], no_pull=ctx.obj["modules_repo_no_pull"], @@ -849,7 +855,6 @@ def lint( ) module_lint.lint( module=tool, - registry=registry, key=key, all_modules=all, print_results=True, diff --git a/nf_core/modules/lint/__init__.py b/nf_core/modules/lint/__init__.py index ba3b3d18fb..81ec32a6ab 100644 --- a/nf_core/modules/lint/__init__.py +++ b/nf_core/modules/lint/__init__.py @@ -69,6 +69,7 @@ def __init__( remote_url=None, branch=None, no_pull=False, + registry=None, hide_progress=False, ): super().__init__( @@ -114,6 +115,7 @@ def __init__( ) for m in self.get_local_components() ] + self.config = nf_core.utils.fetch_wf_config(self.dir, cache_config=True) else: module_dir = Path(self.dir, self.default_modules_path) self.all_remote_modules = [ @@ -124,6 +126,15 @@ def __init__( if not self.all_remote_modules: raise LookupError("No modules in 'modules' directory") + # This could be better, perhaps glob for all nextflow.config files in? + self.config = nf_core.utils.fetch_wf_config(Path(self.dir).joinpath("tests", "config"), cache_config=True) + + if registry is None: + self.registry = self.config.get("docker.registry", "quay.io") + else: + self.registry = registry + log.debug(f"Registry set to {self.registry}") + self.lint_config = None self.modules_json = None @@ -145,7 +156,6 @@ def get_all_lint_tests(is_pipeline): def lint( self, module=None, - registry="quay.io", key=(), all_modules=False, print_results=True, @@ -228,11 +238,11 @@ def lint( # Lint local modules if local and len(local_modules) > 0: - self.lint_modules(local_modules, registry=registry, local=True, fix_version=fix_version) + self.lint_modules(local_modules, local=True, fix_version=fix_version) # Lint nf-core modules if len(remote_modules) > 0: - self.lint_modules(remote_modules, registry=registry, local=False, fix_version=fix_version) + self.lint_modules(remote_modules, local=False, fix_version=fix_version) if print_results: self._print_results(show_passed=show_passed, sort_by=sort_by) @@ -265,13 +275,12 @@ 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, registry="quay.io", local=False, fix_version=False): + def lint_modules(self, modules, 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 """ @@ -292,9 +301,9 @@ def lint_modules(self, modules, registry="quay.io", local=False, fix_version=Fal for mod in modules: progress_bar.update(lint_progress, advance=1, test_name=mod.module_name) - self.lint_module(mod, progress_bar, registry=registry, local=local, fix_version=fix_version) + self.lint_module(mod, progress_bar, local=local, fix_version=fix_version) - def lint_module(self, mod, progress_bar, registry, local=False, fix_version=False): + def lint_module(self, mod, progress_bar, local=False, fix_version=False): """ Perform linting on one module @@ -313,7 +322,7 @@ def lint_module(self, mod, progress_bar, registry, local=False, fix_version=Fals # Only check the main script in case of a local module if local: - self.main_nf(mod, fix_version, registry, progress_bar) + self.main_nf(mod, fix_version, self.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: @@ -325,7 +334,7 @@ def lint_module(self, mod, progress_bar, registry, local=False, fix_version=Fals else: for test_name in self.lint_tests: if test_name == "main_nf": - getattr(self, test_name)(mod, fix_version, registry, progress_bar) + getattr(self, test_name)(mod, fix_version, self.registry, progress_bar) else: getattr(self, test_name)(mod) diff --git a/nf_core/utils.py b/nf_core/utils.py index 3ddce9b870..3d72b7fcb3 100644 --- a/nf_core/utils.py +++ b/nf_core/utils.py @@ -258,7 +258,7 @@ def fetch_wf_config(wf_path, cache_config=True): if cache_basedir and cache_fn: cache_path = os.path.join(cache_basedir, cache_fn) - if os.path.isfile(cache_path): + if os.path.isfile(cache_path) and cache_config is True: log.debug(f"Found a config cache, loading: {cache_path}") with open(cache_path, "r") as fh: try: @@ -274,7 +274,7 @@ def fetch_wf_config(wf_path, cache_config=True): ul = l.decode("utf-8") try: k, v = ul.split(" = ", 1) - config[k] = v + config[k] = v.strip("'\"") except ValueError: log.debug(f"Couldn't find key=value config pair:\n {ul}") @@ -303,6 +303,11 @@ def fetch_wf_config(wf_path, cache_config=True): return config +def parse_nf_config(wf_path, cache_config=True): + config = fetch_wf_config(wf_path=wf_path, cache_config=True) + return config + + def nextflow_cmd(cmd): """Run a Nextflow command and capture the output. Handle errors nicely""" try: From 5d16c043202082d33ac779e2a55f365501c1ea9b Mon Sep 17 00:00:00 2001 From: Adam Talbot Date: Tue, 13 Jun 2023 16:51:57 +0100 Subject: [PATCH 2/5] Update test --- tests/modules/lint.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/modules/lint.py b/tests/modules/lint.py index 5b2c250805..d31f2c3212 100644 --- a/tests/modules/lint.py +++ b/tests/modules/lint.py @@ -85,12 +85,13 @@ def test_modules_lint_multiple_remotes(self): def test_modules_lint_registry(self): """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") + module_lint = nf_core.modules.ModuleLint(dir=self.pipeline_dir, registry="public.ecr.aws") + module_lint.lint(print_results=False, 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 - module_lint.lint(print_results=False, registry="quay.io", module="samtools") + module_lint = nf_core.modules.ModuleLint(dir=self.pipeline_dir) + module_lint.lint(print_results=False, 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 From 1d796dc13ac022a5d634e487cd5a2b4ca5b95169 Mon Sep 17 00:00:00 2001 From: Adam Talbot Date: Wed, 14 Jun 2023 10:17:35 +0100 Subject: [PATCH 3/5] Update test to remove single quotes --- tests/test_download.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_download.py b/tests/test_download.py index 4ee8d3ef6e..332003c426 100644 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -128,7 +128,7 @@ def test_wf_use_local_configs(self, tmp_path): # Test the function download_obj.wf_use_local_configs("workflow") wf_config = nf_core.utils.fetch_wf_config(os.path.join(test_outdir, "workflow"), cache_config=False) - assert wf_config["params.custom_config_base"] == f"'{test_outdir}/workflow/../configs/'" + assert wf_config["params.custom_config_base"] == f"{test_outdir}/workflow/../configs/" # # Tests for 'find_container_images' From 56d0c3626e355fdcc5743dfdd05dab6e85373edf Mon Sep 17 00:00:00 2001 From: Adam Talbot Date: Wed, 14 Jun 2023 10:22:01 +0100 Subject: [PATCH 4/5] better language around help text --- nf_core/__main__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/__main__.py b/nf_core/__main__.py index f531108b1b..32e8fc2700 100644 --- a/nf_core/__main__.py +++ b/nf_core/__main__.py @@ -813,7 +813,7 @@ def create_test_yml(ctx, tool, run_tests, output, force, no_prompts): type=str, metavar="", default=None, - help="Registry to use for containers. If not specified will interpret it from nextflow.config file at attribute docker.registry", + help="Registry to use for containers. If not specified it will use docker.registry value in the nextflow.config file", ) @click.option("-k", "--key", type=str, metavar="", multiple=True, help="Run only these lint tests") @click.option("-a", "--all", is_flag=True, help="Run on all modules") From 0ed72102306907d564b23f2b3063592f1a8df5df Mon Sep 17 00:00:00 2001 From: Adam Talbot Date: Thu, 15 Jun 2023 12:30:06 +0100 Subject: [PATCH 5/5] Remove unused functioN --- nf_core/utils.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/nf_core/utils.py b/nf_core/utils.py index 9fd8626f82..31738edabe 100644 --- a/nf_core/utils.py +++ b/nf_core/utils.py @@ -303,11 +303,6 @@ def fetch_wf_config(wf_path, cache_config=True): return config -def parse_nf_config(wf_path, cache_config=True): - config = fetch_wf_config(wf_path=wf_path, cache_config=True) - return config - - def nextflow_cmd(cmd): """Run a Nextflow command and capture the output. Handle errors nicely""" try: