From 04bd3e49543f475ac211327a0c949f5abe55056d Mon Sep 17 00:00:00 2001 From: Matthias Zepper Date: Tue, 8 Aug 2023 19:13:21 +0200 Subject: [PATCH 1/7] Trim eventual library definitions from container images. --- nf_core/download.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/nf_core/download.py b/nf_core/download.py index cf95d27a14..ef124f2b5a 100644 --- a/nf_core/download.py +++ b/nf_core/download.py @@ -1235,6 +1235,13 @@ def singularity_pull_image(self, container, out_path, cache_path, library, progr Various exceptions possible from `subprocess` execution of Singularity. """ output_path = cache_path or out_path + + # Sometimes, container still contain an explicit library specification, which + # results in attempted pulls e.g. from docker://quay.io/quay.io/qiime2/core:2022.11 + container_parts = container.split("/") + if len(container_parts) > 2: + container = "/".join(container_parts[-2:]) + # Pull using singularity address = f"docker://{library}/{container.replace('docker://', '')}" if shutil.which("singularity"): From 475119a0ab6e9f5d5421b7594bdd414ad311b6d8 Mon Sep 17 00:00:00 2001 From: Matthias Zepper Date: Thu, 17 Aug 2023 14:34:53 +0200 Subject: [PATCH 2/7] Add info message with suggestion how to add the explicit library to the registries to test. --- CHANGELOG.md | 13 +++++++------ nf_core/download.py | 6 ++++++ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c6cfc1fa9..eb09cd911b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ ### Download - Improved container image resolution and prioritization of http downloads over Docker URIs ([#2364](https://github.com/nf-core/tools/pull/2364)). +- Explicit container registry specifications will be ignored, so registries provided with `-l`/`--container-library` are used instead. ([#2403](https://github.com/nf-core/tools/pull/2403)). ### Linting @@ -42,9 +43,9 @@ - `params.max_multiqc_email_size` is no longer required ([#2273](https://github.com/nf-core/tools/pull/2273)) - Remove `cleanup = true` from `test_full.config` in pipeline template ([#2279](https://github.com/nf-core/tools/pull/2279)) - Fix usage docs for specifying `params.yaml` ([#2279](https://github.com/nf-core/tools/pull/2279)) -- Added stub in modules template ([#2277])(https://github.com/nf-core/tools/pull/2277) [Contributed by @nvnieuwk] -- Move registry definitions out of profile scope ([#2286])(https://github.com/nf-core/tools/pull/2286) -- Remove `aws_tower` profile ([#2287])(https://github.com/nf-core/tools/pull/2287) +- Added stub in modules template ([#2277])() [Contributed by @nvnieuwk] +- Move registry definitions out of profile scope ([#2286])() +- Remove `aws_tower` profile ([#2287])() - 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)) - Updates seqeralabs/action-tower-launch to v2.0.0 ([#2301](https://github.com/nf-core/tools/pull/2301)) @@ -85,7 +86,7 @@ _In addition, `-r` / `--revision` has been changed to a parameter that can be pr ### Linting - Warn if container access is denied ([#2270](https://github.com/nf-core/tools/pull/2270)) -- Error if module container specification has quay.io as prefix when it shouldn't have ([#2278])(https://github.com/nf-core/tools/pull/2278/files) +- Error if module container specification has quay.io as prefix when it shouldn't have ([#2278])() - Detect if container is 'simple name' and try to contact quay.io server by default ([#2281](https://github.com/nf-core/tools/pull/2281)) - Warn about null/None/empty default values in `nextflow_schema.json` ([#3328](https://github.com/nf-core/tools/pull/2328)) - Fix linting when creating a pipeline skipping some parts of the template and add CI test ([#2330](https://github.com/nf-core/tools/pull/2330)) @@ -116,7 +117,7 @@ _In addition, `-r` / `--revision` has been changed to a parameter that can be pr - Consistent syntax for branch checks in PRs ([#2202](https://github.com/nf-core/tools/issues/2202)) - Fixed minor Jinja2 templating bug that caused the PR template to miss a newline - Updated AWS tests to use newly moved `seqeralabs/action-tower-launch` instead of `nf-core/tower-action` -- Remove `.cff` files from `.editorconfig` [(#2145)[https://github.com/nf-core/tools/pull/2145]] +- Remove `.cff` files from `.editorconfig` [[#2145](https://github.com/nf-core/tools/pull/2145)] - Simplify pipeline README ([#2186](https://github.com/nf-core/tools/issues/2186)) - Added support for the apptainer container engine via `-profile apptainer`. ([#2244](https://github.com/nf-core/tools/issues/2244)) [Contributed by @jfy133] - Added config `docker.registry` to pipeline template for a configurable default container registry when using Docker containers. Defaults to `quay.io` ([#2133](https://github.com/nf-core/tools/pull/2133)) @@ -1143,7 +1144,7 @@ making a pull-request. See [`.github/CONTRIBUTING.md`](.github/CONTRIBUTING.md) - Move `params`section from `base.config` to `nextflow.config` - Use `env` scope to export `PYTHONNOUSERSITE` in `nextflow.config` to prevent conflicts with host Python environment - Bump minimum Nextflow version to `19.10.0` - required to properly use `env` scope in `nextflow.config` -- Added support for nf-tower in the travis tests, using public mailbox nf-core@mailinator.com +- Added support for nf-tower in the travis tests, using public mailbox - Add link to [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and [Semantic Versioning](http://semver.org/spec/v2.0.0.html) to CHANGELOG - Adjusted `.travis.yml` checks to allow for `patch` branches to be tested - Add Python 3.7 dependency to the `environment.yml` file diff --git a/nf_core/download.py b/nf_core/download.py index ef124f2b5a..dd7e3285aa 100644 --- a/nf_core/download.py +++ b/nf_core/download.py @@ -1238,9 +1238,15 @@ def singularity_pull_image(self, container, out_path, cache_path, library, progr # Sometimes, container still contain an explicit library specification, which # results in attempted pulls e.g. from docker://quay.io/quay.io/qiime2/core:2022.11 + # Thus, we trim whatever precedes the base image specification, but also + # issue a warning about that behavior. container_parts = container.split("/") if len(container_parts) > 2: container = "/".join(container_parts[-2:]) + found_library = container_parts[-3] + log.info( + f'Found explicit container library [bright_magenta]{found_library}[/] in a module. Upon pull failure, retry the download with [bright_magenta] -l "{found_library}"[/]' + ) # Pull using singularity address = f"docker://{library}/{container.replace('docker://', '')}" From 94d2f00dda2d04130439597002dc0dc3f8f0d5cd Mon Sep 17 00:00:00 2001 From: Matthias Zepper Date: Fri, 18 Aug 2023 18:07:53 +0200 Subject: [PATCH 3/7] Suppress container registry info message for common false positives. --- nf_core/download.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/nf_core/download.py b/nf_core/download.py index dd7e3285aa..ad3dbc3dad 100644 --- a/nf_core/download.py +++ b/nf_core/download.py @@ -1244,9 +1244,10 @@ def singularity_pull_image(self, container, out_path, cache_path, library, progr if len(container_parts) > 2: container = "/".join(container_parts[-2:]) found_library = container_parts[-3] - log.info( - f'Found explicit container library [bright_magenta]{found_library}[/] in a module. Upon pull failure, retry the download with [bright_magenta] -l "{found_library}"[/]' - ) + if found_library not in ["", "docker:"]: + log.info( + f'Found explicit container library [bright_magenta]{found_library}[/] in a module. Upon pull failure, retry the download with [bright_magenta] -l "{found_library}"[/]' + ) # Pull using singularity address = f"docker://{library}/{container.replace('docker://', '')}" From d904d4e7cdf3c30c252907d960ceeb69f40ad839 Mon Sep 17 00:00:00 2001 From: Matthias Zepper Date: Thu, 31 Aug 2023 12:46:03 +0200 Subject: [PATCH 4/7] Different handling for container images: Ignore -l parameter for absolute URIs instead of trimming explicit registries. --- nf_core/download.py | 54 ++++++++++++++++++++++++++++----------------- 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/nf_core/download.py b/nf_core/download.py index ad3dbc3dad..9ca786b5e3 100644 --- a/nf_core/download.py +++ b/nf_core/download.py @@ -1081,7 +1081,10 @@ def get_singularity_images(self, current_revision=""): continue except ContainerError.ImageNotFound as e: # Try other registries - continue + if e.error_log.absoluteURI: + break # there no point in trying other registries if absolute URI was specified. + else: + continue except ContainerError.InvalidTag as e: # Try other registries continue @@ -1089,7 +1092,10 @@ def get_singularity_images(self, current_revision=""): # Try other registries log.error(e.message) log.error(e.helpmessage) - continue + if e.error_log.absoluteURI: + break # there no point in trying other registries if absolute URI was specified. + else: + continue else: # The else clause executes after the loop completes normally. # This means the library loop completed without breaking, indicating failure for all libraries (registries) @@ -1237,20 +1243,16 @@ def singularity_pull_image(self, container, out_path, cache_path, library, progr output_path = cache_path or out_path # Sometimes, container still contain an explicit library specification, which - # results in attempted pulls e.g. from docker://quay.io/quay.io/qiime2/core:2022.11 - # Thus, we trim whatever precedes the base image specification, but also - # issue a warning about that behavior. + # resulted in attempted pulls e.g. from docker://quay.io/quay.io/qiime2/core:2022.11 + # Thus, if an explicit registry is specified, the provided -l value is ignored. container_parts = container.split("/") if len(container_parts) > 2: - container = "/".join(container_parts[-2:]) - found_library = container_parts[-3] - if found_library not in ["", "docker:"]: - log.info( - f'Found explicit container library [bright_magenta]{found_library}[/] in a module. Upon pull failure, retry the download with [bright_magenta] -l "{found_library}"[/]' - ) + address = container + absolute_URI = True + else: + address = f"docker://{library}/{container.replace('docker://', '')}" + absolute_URI = False - # Pull using singularity - address = f"docker://{library}/{container.replace('docker://', '')}" if shutil.which("singularity"): singularity_command = ["singularity", "pull", "--name", output_path, address] elif shutil.which("apptainer"): @@ -1284,6 +1286,7 @@ def singularity_pull_image(self, container, out_path, cache_path, library, progr container=container, registry=library, address=address, + absolute_URI=absolute_URI, out_path=out_path if out_path else cache_path or "", singularity_command=singularity_command, error_msg=lines, @@ -1569,10 +1572,11 @@ def bare_clone(self, destination): class ContainerError(Exception): """A class of errors related to pulling containers with Singularity/Apptainer""" - def __init__(self, container, registry, address, out_path, singularity_command, error_msg): + def __init__(self, container, registry, address, absolute_URI, out_path, singularity_command, error_msg): self.container = container self.registry = registry self.address = address + self.absolute_URI = absolute_URI self.out_path = out_path self.singularity_command = singularity_command self.error_msg = error_msg @@ -1629,10 +1633,15 @@ class ImageNotFound(FileNotFoundError): def __init__(self, error_log): self.error_log = error_log - self.message = ( - f'[bold red]"Pulling "{self.error_log.container}" from "{self.error_log.address}" failed.[/]\n' - ) - self.helpmessage = f'Saving image of "{self.error_log.container}" failed.\nPlease troubleshoot the command \n"{" ".join(self.error_log.singularity_command)}" manually.f\n' + if not self.error_log.absolute_URI: + self.message = ( + f'[bold red]"Pulling "{self.error_log.container}" from "{self.error_log.address}" failed.[/]\n' + ) + self.helpmessage = f'Saving image of "{self.error_log.container}" failed.\nPlease troubleshoot the command \n"{" ".join(self.error_log.singularity_command)}" manually.f\n' + else: + self.message = f'[bold red]"The pipeline requested the download of non-existing container image "{self.error_log.address}"[/]\n' + self.helpmessage = f'Please try to rerun \n"{" ".join(self.error_log.singularity_command)}" manually with a different registry.f\n' + super().__init__(self.message) class InvalidTag(AttributeError): @@ -1660,6 +1669,11 @@ class OtherError(RuntimeError): def __init__(self, error_log): self.error_log = error_log - self.message = f'[bold red]"{self.error_log.container}" failed for unclear reasons.[/]\n' - self.helpmessage = f'Pulling of "{self.error_log.container}" failed.\nPlease troubleshoot the command \n"{" ".join(self.error_log.singularity_command)}" manually.\n' + if not self.error_log.absolute_URI: + self.message = f'[bold red]"{self.error_log.container}" failed for unclear reasons.[/]\n' + self.helpmessage = f'Pulling of "{self.error_log.container}" failed.\nPlease troubleshoot the command \n"{" ".join(self.error_log.singularity_command)}" manually.\n' + else: + self.message = f'[bold red]"The pipeline requested the download of non-existing container image "{self.error_log.address}"[/]\n' + self.helpmessage = f'Please try to rerun \n"{" ".join(self.error_log.singularity_command)}" manually with a different registry.f\n' + super().__init__(self.message, self.helpmessage, self.error_log) From 976ddf444ccc9ec2073e4784c75095e0ad438a29 Mon Sep 17 00:00:00 2001 From: Matthias Zepper Date: Thu, 31 Aug 2023 13:14:06 +0200 Subject: [PATCH 5/7] Stop running Pytests or pushing new API docs if only the changelog was updated. --- .github/workflows/pytest.yml | 6 ++++++ .github/workflows/tools-api-docs-dev.yml | 4 ++++ CHANGELOG.md | 2 +- 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index 10d0d7dc02..507962edf7 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -5,7 +5,13 @@ on: push: branches: - dev + paths-ignore: + - 'docs/**' + - 'CHANGELOG.md' pull_request: + paths-ignore: + - 'docs/**' + - 'CHANGELOG.md' release: types: [published] diff --git a/.github/workflows/tools-api-docs-dev.yml b/.github/workflows/tools-api-docs-dev.yml index e52eee1331..a2bbb06d85 100644 --- a/.github/workflows/tools-api-docs-dev.yml +++ b/.github/workflows/tools-api-docs-dev.yml @@ -4,7 +4,11 @@ on: push: branches: - dev + paths-ignore: + - 'CHANGELOG.md' pull_request: + paths-ignore: + - 'CHANGELOG.md' release: types: [published] diff --git a/CHANGELOG.md b/CHANGELOG.md index eb09cd911b..d41ec23520 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ ### Download - Improved container image resolution and prioritization of http downloads over Docker URIs ([#2364](https://github.com/nf-core/tools/pull/2364)). -- Explicit container registry specifications will be ignored, so registries provided with `-l`/`--container-library` are used instead. ([#2403](https://github.com/nf-core/tools/pull/2403)). +- Registries provided with `-l`/`--container-library` will be ignored for modules with explicit container registry specifications ([#2403](https://github.com/nf-core/tools/pull/2403)). ### Linting From d27c4454b7866748b0e5de933df6ba0b5efad7f0 Mon Sep 17 00:00:00 2001 From: Matthias Zepper Date: Thu, 31 Aug 2023 13:20:41 +0200 Subject: [PATCH 6/7] Run prettier on Github Actions. --- .github/workflows/pytest.yml | 10 +++++----- .github/workflows/tools-api-docs-dev.yml | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index 507962edf7..2bd49f9f00 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -6,12 +6,12 @@ on: branches: - dev paths-ignore: - - 'docs/**' - - 'CHANGELOG.md' + - "docs/**" + - "CHANGELOG.md" pull_request: - paths-ignore: - - 'docs/**' - - 'CHANGELOG.md' + paths-ignore: + - "docs/**" + - "CHANGELOG.md" release: types: [published] diff --git a/.github/workflows/tools-api-docs-dev.yml b/.github/workflows/tools-api-docs-dev.yml index a2bbb06d85..8ce4fc60bf 100644 --- a/.github/workflows/tools-api-docs-dev.yml +++ b/.github/workflows/tools-api-docs-dev.yml @@ -5,10 +5,10 @@ on: branches: - dev paths-ignore: - - 'CHANGELOG.md' + - "CHANGELOG.md" pull_request: paths-ignore: - - 'CHANGELOG.md' + - "CHANGELOG.md" release: types: [published] From 7573c90c2035139ccb248c5e5acbbf37b0cfe62b Mon Sep 17 00:00:00 2001 From: Matthias Zepper <6963520+MatthiasZepper@users.noreply.github.com> Date: Fri, 1 Sep 2023 19:37:21 +0200 Subject: [PATCH 7/7] Undo prettier's sins in CHANGELOG.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Júlia Mir Pedrol --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d41ec23520..4c406f9a41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,7 +43,7 @@ - `params.max_multiqc_email_size` is no longer required ([#2273](https://github.com/nf-core/tools/pull/2273)) - Remove `cleanup = true` from `test_full.config` in pipeline template ([#2279](https://github.com/nf-core/tools/pull/2279)) - Fix usage docs for specifying `params.yaml` ([#2279](https://github.com/nf-core/tools/pull/2279)) -- Added stub in modules template ([#2277])() [Contributed by @nvnieuwk] +- Added stub in modules template ([#2277](https://github.com/nf-core/tools/pull/2277)) [Contributed by @nvnieuwk] - Move registry definitions out of profile scope ([#2286])() - Remove `aws_tower` profile ([#2287])() - Fixed the Slack report to include the pipeline name ([#2291](https://github.com/nf-core/tools/pull/2291))