From 6ab6c8ddb3a8a347e506e3a7833d7baec55bb044 Mon Sep 17 00:00:00 2001 From: Matthias Zepper Date: Tue, 23 Apr 2024 14:57:22 +0200 Subject: [PATCH 01/16] Update Docstring of DownloadWorkflow class. --- nf_core/download.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/nf_core/download.py b/nf_core/download.py index 8ffecc0f5..2978dd08a 100644 --- a/nf_core/download.py +++ b/nf_core/download.py @@ -88,10 +88,17 @@ class DownloadWorkflow: Args: pipeline (str): A nf-core pipeline name. - revision (List[str]): The workflow revision to download, like `1.0`. Defaults to None. - container (bool): Flag, if the Singularity container should be downloaded as well. Defaults to False. - platform (bool): Flag, to customize the download for Seqera Platform (convert to git bare repo). Defaults to False. + revision (List[str]): The workflow revision(s) to download, like `1.0` or `dev` . Defaults to None. outdir (str): Path to the local download directory. Defaults to None. + compress_type (str): Type of compression for the downloaded files. Defaults to None. + force (bool): Flag to force download even if files already exist (overwrite existing files). Defaults to False. + platform (bool): Flag to customize the download for Seqera Platform (convert to git bare repo). Defaults to False. + download_configuration (str): Download the configuration files from nf-core/configs. Defaults to None. + container_system (str): The container system to use (e.g., "singularity"). Defaults to None. + container_library (str): The container libraries (registries) to use. Defaults to None. + container_cache_utilisation (str): If a local or remote cache of already existing container images should be considered. Defaults to None. + container_cache_index (str): An index for the remote container cache. Defaults to None. + parallel_downloads (int): The number of parallel downloads to use. Defaults to 4. """ def __init__( From d2f8525c7abb89e658ce747e11a5f0483a3a805b Mon Sep 17 00:00:00 2001 From: Matthias Zepper Date: Tue, 23 Apr 2024 17:22:16 +0200 Subject: [PATCH 02/16] Include the '--additional_tags' argument in the CLI for 'nf-core download' --- nf_core/__main__.py | 10 +++++++++- tests/test_cli.py | 2 ++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/nf_core/__main__.py b/nf_core/__main__.py index 807bc776b..f3212200d 100644 --- a/nf_core/__main__.py +++ b/nf_core/__main__.py @@ -372,7 +372,7 @@ def create_params_file(pipeline, revision, output, force, show_hidden): is_flag=True, default=False, hidden=True, - help="Download for Seqera Platform. DEPRECATED: Please use --platform instead.", + help="Download for Seqera Platform. DEPRECATED: Please use `--platform` instead.", ) @click.option( "-t", @@ -388,6 +388,12 @@ def create_params_file(pipeline, revision, output, force, show_hidden): default=False, help="Include configuration profiles in download. Not available with `--platform`", ) +@click.option( + "-a", + "--additional-tags", + multiple=True, + help="Add custom alias tags to `--platform` downloads. For example, '-a \"3.10=validated\"' adds the custom 'validated' tag to the 3.10 release.", +) # -c changed to -s for consistency with other --container arguments, where it is always the first letter of the last word. # Also -c might be used instead of -d for config in a later release, but reusing params for different options in two subsequent releases might be too error-prone. @click.option( @@ -430,6 +436,7 @@ def download( tower, platform, download_configuration, + additional_tags, container_system, container_library, container_cache_utilisation, @@ -452,6 +459,7 @@ def download( force, tower or platform, # True if either specified download_configuration, + additional_tags, container_system, container_library, container_cache_utilisation, diff --git a/tests/test_cli.py b/tests/test_cli.py index 913a4aac1..826008f03 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -168,6 +168,7 @@ def test_cli_download(self, mock_dl): "force": None, "platform": None, "download-configuration": None, + "additional-tags": "3.12=testing", "container-system": "singularity", "container-library": "quay.io", "container-cache-utilisation": "copy", @@ -188,6 +189,7 @@ def test_cli_download(self, mock_dl): "force" in params, "platform" in params, "download-configuration" in params, + (params["additional-tags"],), params["container-system"], (params["container-library"],), params["container-cache-utilisation"], From 550da8650fd0a893f1882b7e6c9cc6cb13c77615 Mon Sep 17 00:00:00 2001 From: Matthias Zepper Date: Tue, 23 Apr 2024 18:15:04 +0200 Subject: [PATCH 03/16] Started with additional_tags handling in the level of the DownloadWorkflow class. ToDo for WorkflowRepo class. --- nf_core/download.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/nf_core/download.py b/nf_core/download.py index 2978dd08a..d12b498b2 100644 --- a/nf_core/download.py +++ b/nf_core/download.py @@ -94,8 +94,9 @@ class DownloadWorkflow: force (bool): Flag to force download even if files already exist (overwrite existing files). Defaults to False. platform (bool): Flag to customize the download for Seqera Platform (convert to git bare repo). Defaults to False. download_configuration (str): Download the configuration files from nf-core/configs. Defaults to None. + additional_tags (List[str]): Specify additional tags to add to the downloaded pipeline. Defaults to None. container_system (str): The container system to use (e.g., "singularity"). Defaults to None. - container_library (str): The container libraries (registries) to use. Defaults to None. + container_library (List[str]): The container libraries (registries) to use. Defaults to None. container_cache_utilisation (str): If a local or remote cache of already existing container images should be considered. Defaults to None. container_cache_index (str): An index for the remote container cache. Defaults to None. parallel_downloads (int): The number of parallel downloads to use. Defaults to 4. @@ -110,6 +111,7 @@ def __init__( force=False, platform=False, download_configuration=None, + additional_tags=None, container_system=None, container_library=None, container_cache_utilisation=None, @@ -132,6 +134,15 @@ def __init__( # this implies that non-interactive "no" choice is only possible implicitly (e.g. with --platform or if prompt is suppressed by !stderr.is_interactive). # only alternative would have been to make it a parameter with argument, e.g. -d="yes" or -d="no". self.include_configs = True if download_configuration else False if bool(platform) else None + # Additional tags to add to the downloaded pipeline. This enables to mark particular commits or revisions with + # additional tags, e.g. "stable", "testing", "validated", "production" etc. Since this requires a git-repo, it is only + # available for the bare / Seqera Platform download. + if isinstance(additional_tags, str) and bool(len(additional_tags)) and self.platform: + self.additional_tags = [additional_tags] + elif isinstance(additional_tags, tuple) and bool(len(additional_tags)) and self.platform: + self.additional_tags = [*additional_tags] + else: + self.additional_tags = None # Specifying a cache index or container library implies that containers should be downloaded. self.container_system = "singularity" if container_cache_index or bool(container_library) else container_system # Manually specified container library (registry) @@ -289,6 +300,7 @@ def download_workflow_platform(self, location=None): remote_url=f"https://github.com/{self.pipeline}.git", revision=self.revision if self.revision else None, commit=self.wf_sha.values() if bool(self.wf_sha) else None, + additional_tags=self.additional_tags if self.additional_tags else None, location=(location if location else None), # manual location is required for the tests to work in_cache=False, ) @@ -1496,6 +1508,7 @@ def __init__( remote_url, revision, commit, + additional_tags, location=None, hide_progress=False, in_cache=True, @@ -1533,6 +1546,9 @@ def __init__( # expose some instance attributes self.tags = self.repo.tags + # additional tags to be added to the repository + self.additional_tags = additional_tags + def __repr__(self): """Called by print, creates representation of object""" return f"" From 620ca82da9c375a872f656c05f098c7bbd42000c Mon Sep 17 00:00:00 2001 From: Matthias Zepper Date: Wed, 24 Apr 2024 17:21:56 +0200 Subject: [PATCH 04/16] Implement a private __add_additional_tags() method for the WorkflowRepo class in 'nf-core download'. --- nf_core/download.py | 38 ++++++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/nf_core/download.py b/nf_core/download.py index d12b498b2..184a02d32 100644 --- a/nf_core/download.py +++ b/nf_core/download.py @@ -308,6 +308,8 @@ def download_workflow_platform(self, location=None): # Remove tags for those revisions that had not been selected self.workflow_repo.tidy_tags_and_branches() + import pdb; pdb.set_trace() + # create a bare clone of the modified repository needed for Seqera Platform self.workflow_repo.bare_clone(os.path.join(self.outdir, self.output_filename)) @@ -1543,16 +1545,21 @@ def __init__( self.setup_local_repo(remote=remote_url, location=location, in_cache=in_cache) - # expose some instance attributes - self.tags = self.repo.tags - # additional tags to be added to the repository - self.additional_tags = additional_tags + self.additional_tags = additional_tags if additional_tags else None def __repr__(self): """Called by print, creates representation of object""" return f"" + @property + def heads(self): + return self.repo.heads + + @property + def tags(self): + return self.repo.tags + def access(self): if os.path.exists(self.local_repo_dir): return self.local_repo_dir @@ -1663,7 +1670,6 @@ def tidy_tags_and_branches(self): # delete unwanted tags from repository for tag in tags_to_remove: self.repo.delete_tag(tag) - self.tags = self.repo.tags # switch to a revision that should be kept, because deleting heads fails, if they are checked out (e.g. "master") self.checkout(self.revision[0]) @@ -1700,7 +1706,8 @@ def tidy_tags_and_branches(self): if self.repo.head.is_detached: self.repo.head.reset(index=True, working_tree=True) - self.heads = self.repo.heads + # Apply the custom additional tags to the repository + self.__add_additional_tags() # get all tags and available remote_branches completed_revisions = {revision.name for revision in self.repo.heads + self.repo.tags} @@ -1718,6 +1725,25 @@ def tidy_tags_and_branches(self): self.retry_setup_local_repo(skip_confirm=True) raise DownloadError(e) from e + # "Private" method to add the additional custom tags to the repository. + def __add_additional_tags(self) -> None: + if self.additional_tags: + for additional_tag in self.additional_tags: + # A valid git branch or tag name can contain alphanumeric characters, underscores, hyphens, and dots. + # But it must not start with a dot, hyphen or underscore and also cannot contain two consecutive dots. + if re.match(r"^\w[\w_.-]+={1}\w[\w_.-]+$",additional_tag) and '..' not in additional_tag: + anchor, tag = additional_tag.split("=") + if self.repo.is_valid_object(anchor) and not self.repo.is_valid_object(tag): + try: + self.repo.create_tag(tag,ref=anchor,message=f"Synonynmous tag to {anchor}; added by 'nf-core download'.") + self.repo.create_head(tag,anchor) + except (GitCommandError, InvalidGitRepositoryError) as e: + log.error(f"[red]Additional tag(s) could not be applied:[/]\n{e}\n") + else: + log.error(f"[red]Adding the additional tag '{tag}' to '{anchor}' failed.[/]\n Mind that '{anchor}' must be a valid git reference that resolves to a commit, while '{tag}' must not exist hitherto.") + else: + log.error(f"[red]Could not apply invalid '-a' / '--additional-tag' specification[/]: '{additional_tag}'") + def bare_clone(self, destination): if self.repo: try: From da0eab10ecf079ea44f85e7b4d96cd8bd5a06721 Mon Sep 17 00:00:00 2001 From: Matthias Zepper Date: Thu, 25 Apr 2024 21:02:54 +0200 Subject: [PATCH 05/16] Experiment with capturing log messages in tests. --- nf_core/download.py | 4 +-- tests/test_download.py | 61 +++++++++++++++++++++++++++++++++++++++++- tests/utils.py | 52 ++++++++++++++++++++++++++++++++++- 3 files changed, 112 insertions(+), 5 deletions(-) diff --git a/nf_core/download.py b/nf_core/download.py index 184a02d32..62fe08e7d 100644 --- a/nf_core/download.py +++ b/nf_core/download.py @@ -308,8 +308,6 @@ def download_workflow_platform(self, location=None): # Remove tags for those revisions that had not been selected self.workflow_repo.tidy_tags_and_branches() - import pdb; pdb.set_trace() - # create a bare clone of the modified repository needed for Seqera Platform self.workflow_repo.bare_clone(os.path.join(self.outdir, self.output_filename)) @@ -1736,7 +1734,7 @@ def __add_additional_tags(self) -> None: if self.repo.is_valid_object(anchor) and not self.repo.is_valid_object(tag): try: self.repo.create_tag(tag,ref=anchor,message=f"Synonynmous tag to {anchor}; added by 'nf-core download'.") - self.repo.create_head(tag,anchor) + self.repo.create_head(tag,anchor) # should heads be created as well? except (GitCommandError, InvalidGitRepositoryError) as e: log.error(f"[red]Additional tag(s) could not be applied:[/]\n{e}\n") else: diff --git a/tests/test_download.py b/tests/test_download.py index 14a96be26..0fc3abae6 100644 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -16,7 +16,7 @@ from nf_core.synced_repo import SyncedRepo from nf_core.utils import run_cmd -from .utils import with_temporary_folder +from .utils import with_logger, with_temporary_folder class DownloadTest(unittest.TestCase): @@ -623,3 +623,62 @@ def test_download_workflow_for_platform(self, tmp_dir, _): "https://depot.galaxyproject.org/singularity/mulled-v2-1fa26d1ce03c295fe2fdcf85831a92fbcbd7e8c2:59cdd445419f14abac76b31dd0d71217994cbcc9-0" in download_obj.containers ) # indirect definition via $container variable. + + + # + # Test adding custom tags to Seqera Platform download + # + @with_temporary_folder + @mock.patch("nf_core.download.DownloadWorkflow.get_singularity_images") + @with_logger + def test_download_workflow_for_platform_with_custom_tags(self, tmp_dir, _, logger): + + from git.refs.head import Head + from git.refs.tag import TagReference + + download_obj = DownloadWorkflow( + pipeline="nf-core/rnaseq", + revision=("3.7", "3.9"), + compress_type="none", + platform=True, + container_system=None, + additional_tags=("3.7=a.tad.outdated", "3.9=cool_revision", "3.9=invalid tag","3.14.0=not_included","What is this?"), + ) + + download_obj.include_configs = False # suppress prompt, because stderr.is_interactive doesn't. + + assert isinstance(download_obj.revision, list) and len(download_obj.revision) == 2 + assert isinstance(download_obj.wf_sha, dict) and len(download_obj.wf_sha) == 0 + assert isinstance(download_obj.wf_download_url, dict) and len(download_obj.wf_download_url) == 0 + assert isinstance(download_obj.additional_tags, list) and len(download_obj.additional_tags) == 5 + + wfs = nf_core.list.Workflows() + wfs.get_remote_workflows() + ( + download_obj.pipeline, + download_obj.wf_revisions, + download_obj.wf_branches, + ) = nf_core.utils.get_repo_releases_branches(download_obj.pipeline, wfs) + + download_obj.get_revision_hash() + download_obj.output_filename = f"{download_obj.outdir}.git" + download_obj.download_workflow_platform(location=tmp_dir) + + assert download_obj.workflow_repo + assert isinstance(download_obj.workflow_repo, WorkflowRepo) + assert issubclass(type(download_obj.workflow_repo), SyncedRepo) + + # assert that every additional tag has been passed on to the WorkflowRepo instance + assert download_obj.additional_tags == download_obj.workflow_repo.additional_tags + + # assert that the additional tags are all TagReference objects + assert all(isinstance(tag, TagReference) for tag in download_obj.workflow_repo.tags) + + workflow_repo_tags = {tag.name for tag in download_obj.workflow_repo.tags} + assert len(workflow_repo_tags) == 4 + # the invalid/malformed additional_tags should not have been added. + assert all(tag in workflow_repo_tags for tag in {'3.7', 'a.tad.outdated', 'cool_revision', '3.9'}) + assert not any(tag in workflow_repo_tags for tag in {'invalid tag','not_included','What is this?'}) + + import pdb; pdb.set_trace() + diff --git a/tests/utils.py b/tests/utils.py index 89c132881..c87e2c5ec 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -1,7 +1,7 @@ """ Helper functions for tests """ - +from logging import getLogger, Logger, LogRecord, Handler import functools import os import tempfile @@ -56,6 +56,56 @@ def wrapper(*args: Any, **kwargs: Any) -> Any: return wrapper +class FirstHandlerProxy: + """ + A proxy so the in operator can be used directly on the logger for convenience in asserting the test log messages. + """ + def __init__(self, logger: Logger) -> None: + self.logger = logger + + def __contains__(self, message: str) -> bool: + return message in self.logger.handlers[0] + + def get_log_messages(self) -> list[str]: + return self.logger.handlers[0].log_messages + +class LogToList(Handler): + """ + A logging handler that appends log messages to a list for simple log message capturing. + """ + def __init__(self) -> None: + super().__init__() + self.log_messages = [] + + def __repr__(self) -> str: + return str(self.log_messages) + + # enable using the in operator directly on the handler + def __contains__(self, message: str) -> bool: + return message in self.log_messages + + def emit(self, record: LogRecord) -> None: + self.log_messages.append(self.format(record)) + +def with_logger(func: Callable[..., Any]) -> Callable[..., Any]: + """ + Captures the log messages from a decorated test into a dictionary. + Pass the logger to the decorated function and assert with the _in_ operator. + + assert 'This is a log message' in test_logger + """ + + @functools.wraps(func) + def wrapper(*args: Any, **kwargs: Any) -> Any: + logger = getLogger(func.__name__) + list_handler = LogToList() + logger.addHandler(list_handler) + try: + return func(*args, FirstHandlerProxy(logger), **kwargs) + finally: + logger.removeHandler(list_handler) + + return wrapper def mock_anaconda_api_calls(rsps: responses.RequestsMock, module: str, version: str) -> None: """Mock anaconda api calls for module""" From 113c9d03990aca415e0e7cb315f03a72723e84b9 Mon Sep 17 00:00:00 2001 From: Matthias Zepper Date: Fri, 26 Apr 2024 19:00:40 +0200 Subject: [PATCH 06/16] Finally managed to assert over log messages, thanks @adamrtalbot . --- tests/test_download.py | 107 ++++++++++++++++++++++++----------------- tests/utils.py | 53 +------------------- 2 files changed, 64 insertions(+), 96 deletions(-) diff --git a/tests/test_download.py b/tests/test_download.py index 0fc3abae6..6928a9087 100644 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -1,5 +1,6 @@ """Tests for the download subcommand of nf-core tools""" +import logging import os import re import shutil @@ -16,10 +17,29 @@ from nf_core.synced_repo import SyncedRepo from nf_core.utils import run_cmd -from .utils import with_logger, with_temporary_folder +from .utils import with_temporary_folder class DownloadTest(unittest.TestCase): + + @pytest.fixture(autouse=True) + def use_caplog(self, caplog): + self._caplog = caplog + + @property + def logged_levels(self) -> list[str]: + return [record.levelname for record in self._caplog.records] + + @property + def logged_messages(self) -> list[str]: + return [record.message for record in self._caplog.records] + + def __contains__(self, item: str) -> bool: + """Allows to check for log messages easily using the in operator inside a test: + assert 'my log message' in self + """ + return any(record.message == item for record in self._caplog.records if self._caplog) + # # Tests for 'get_release_hash' # @@ -628,57 +648,56 @@ def test_download_workflow_for_platform(self, tmp_dir, _): # # Test adding custom tags to Seqera Platform download # - @with_temporary_folder @mock.patch("nf_core.download.DownloadWorkflow.get_singularity_images") - @with_logger - def test_download_workflow_for_platform_with_custom_tags(self, tmp_dir, _, logger): - - from git.refs.head import Head - from git.refs.tag import TagReference - - download_obj = DownloadWorkflow( - pipeline="nf-core/rnaseq", - revision=("3.7", "3.9"), - compress_type="none", - platform=True, - container_system=None, - additional_tags=("3.7=a.tad.outdated", "3.9=cool_revision", "3.9=invalid tag","3.14.0=not_included","What is this?"), - ) + @with_temporary_folder + def test_download_workflow_for_platform_with_custom_tags(self,_, tmp_dir): + with self._caplog.at_level(logging.INFO): + from git.refs.head import Head + from git.refs.tag import TagReference + + download_obj = DownloadWorkflow( + pipeline="nf-core/rnaseq", + revision=("3.7", "3.9"), + compress_type="none", + platform=True, + container_system=None, + additional_tags=("3.7=a.tad.outdated", "3.9=cool_revision", "3.9=invalid tag","3.14.0=not_included","What is this?"), + ) - download_obj.include_configs = False # suppress prompt, because stderr.is_interactive doesn't. + download_obj.include_configs = False # suppress prompt, because stderr.is_interactive doesn't. - assert isinstance(download_obj.revision, list) and len(download_obj.revision) == 2 - assert isinstance(download_obj.wf_sha, dict) and len(download_obj.wf_sha) == 0 - assert isinstance(download_obj.wf_download_url, dict) and len(download_obj.wf_download_url) == 0 - assert isinstance(download_obj.additional_tags, list) and len(download_obj.additional_tags) == 5 + assert isinstance(download_obj.revision, list) and len(download_obj.revision) == 2 + assert isinstance(download_obj.wf_sha, dict) and len(download_obj.wf_sha) == 0 + assert isinstance(download_obj.wf_download_url, dict) and len(download_obj.wf_download_url) == 0 + assert isinstance(download_obj.additional_tags, list) and len(download_obj.additional_tags) == 5 - wfs = nf_core.list.Workflows() - wfs.get_remote_workflows() - ( - download_obj.pipeline, - download_obj.wf_revisions, - download_obj.wf_branches, - ) = nf_core.utils.get_repo_releases_branches(download_obj.pipeline, wfs) + wfs = nf_core.list.Workflows() + wfs.get_remote_workflows() + ( + download_obj.pipeline, + download_obj.wf_revisions, + download_obj.wf_branches, + ) = nf_core.utils.get_repo_releases_branches(download_obj.pipeline, wfs) - download_obj.get_revision_hash() - download_obj.output_filename = f"{download_obj.outdir}.git" - download_obj.download_workflow_platform(location=tmp_dir) + download_obj.get_revision_hash() + download_obj.output_filename = f"{download_obj.outdir}.git" + download_obj.download_workflow_platform(location=tmp_dir) - assert download_obj.workflow_repo - assert isinstance(download_obj.workflow_repo, WorkflowRepo) - assert issubclass(type(download_obj.workflow_repo), SyncedRepo) + assert download_obj.workflow_repo + assert isinstance(download_obj.workflow_repo, WorkflowRepo) + assert issubclass(type(download_obj.workflow_repo), SyncedRepo) - # assert that every additional tag has been passed on to the WorkflowRepo instance - assert download_obj.additional_tags == download_obj.workflow_repo.additional_tags + # assert that every additional tag has been passed on to the WorkflowRepo instance + assert download_obj.additional_tags == download_obj.workflow_repo.additional_tags - # assert that the additional tags are all TagReference objects - assert all(isinstance(tag, TagReference) for tag in download_obj.workflow_repo.tags) + # assert that the additional tags are all TagReference objects + assert all(isinstance(tag, TagReference) for tag in download_obj.workflow_repo.tags) - workflow_repo_tags = {tag.name for tag in download_obj.workflow_repo.tags} - assert len(workflow_repo_tags) == 4 - # the invalid/malformed additional_tags should not have been added. - assert all(tag in workflow_repo_tags for tag in {'3.7', 'a.tad.outdated', 'cool_revision', '3.9'}) - assert not any(tag in workflow_repo_tags for tag in {'invalid tag','not_included','What is this?'}) + workflow_repo_tags = {tag.name for tag in download_obj.workflow_repo.tags} + assert len(workflow_repo_tags) == 4 + # the invalid/malformed additional_tags should not have been added. + assert all(tag in workflow_repo_tags for tag in {'3.7', 'a.tad.outdated', 'cool_revision', '3.9'}) + assert not any(tag in workflow_repo_tags for tag in {'invalid tag','not_included','What is this?'}) - import pdb; pdb.set_trace() + assert all(log in self.logged_messages for log in {"[red]Could not apply invalid '-a' / '--additional-tag' specification[/]: '3.9=invalid tag'", "[red]Adding the additional tag 'not_included' to '3.14.0' failed.[/]\n Mind that '3.14.0' must be a valid git reference that resolves to a commit, while 'not_included' must not exist hitherto.", "[red]Could not apply invalid '-a' / '--additional-tag' specification[/]: 'What is this?'"}) diff --git a/tests/utils.py b/tests/utils.py index c87e2c5ec..4635c53b7 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -1,7 +1,7 @@ """ Helper functions for tests """ -from logging import getLogger, Logger, LogRecord, Handler + import functools import os import tempfile @@ -56,57 +56,6 @@ def wrapper(*args: Any, **kwargs: Any) -> Any: return wrapper -class FirstHandlerProxy: - """ - A proxy so the in operator can be used directly on the logger for convenience in asserting the test log messages. - """ - def __init__(self, logger: Logger) -> None: - self.logger = logger - - def __contains__(self, message: str) -> bool: - return message in self.logger.handlers[0] - - def get_log_messages(self) -> list[str]: - return self.logger.handlers[0].log_messages - -class LogToList(Handler): - """ - A logging handler that appends log messages to a list for simple log message capturing. - """ - def __init__(self) -> None: - super().__init__() - self.log_messages = [] - - def __repr__(self) -> str: - return str(self.log_messages) - - # enable using the in operator directly on the handler - def __contains__(self, message: str) -> bool: - return message in self.log_messages - - def emit(self, record: LogRecord) -> None: - self.log_messages.append(self.format(record)) - -def with_logger(func: Callable[..., Any]) -> Callable[..., Any]: - """ - Captures the log messages from a decorated test into a dictionary. - Pass the logger to the decorated function and assert with the _in_ operator. - - assert 'This is a log message' in test_logger - """ - - @functools.wraps(func) - def wrapper(*args: Any, **kwargs: Any) -> Any: - logger = getLogger(func.__name__) - list_handler = LogToList() - logger.addHandler(list_handler) - try: - return func(*args, FirstHandlerProxy(logger), **kwargs) - finally: - logger.removeHandler(list_handler) - - return wrapper - def mock_anaconda_api_calls(rsps: responses.RequestsMock, module: str, version: str) -> None: """Mock anaconda api calls for module""" anaconda_api_url = f"https://api.anaconda.org/package/bioconda/{module}" From 35b4fd66e03e36411924e17b7864ed3ceeb56f6f Mon Sep 17 00:00:00 2001 From: Matthias Zepper Date: Fri, 26 Apr 2024 19:40:56 +0200 Subject: [PATCH 07/16] Update the README.md to reflect the '-a' / '--additional-tags' argument of 'nf-core download'. --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index a5e679986..65cfc21b3 100644 --- a/README.md +++ b/README.md @@ -437,6 +437,8 @@ Subsequently, the `*.git` folder can be moved to it's final destination and link > [!TIP] > Also without access to Seqera Platform, pipelines downloaded with the `--platform` flag can be run if the _absolute_ path is specified: `nextflow run -r 2.5 file:/path/to/pipelinedownload.git`. Downloads in this format allow you to include multiple revisions of a pipeline in a single file, but require that the revision (e.g. `-r 2.5`) is always explicitly specified. +Facilities and those who are setting up pipelines for others to use may find the '-a' / '--additional-tag' argument helpful. It must be followed by a string in a "key=value" format and can be provided multiple times. The left-hand side must refer to a valid branch, tag or commit SHA. The right-hand side must comply with the naming conventions for Git tags and may not yet exist in the repository. The '-a' / '--additional-tag' argument allows to customize the downloaded pipeline with additional tags that can be used to select particular revisions in the Seqera Platform interface. For example, an accredited facility may opt to tag particular revisions according to their structured release management process: '-a "3.12.0=testing" -a "3.9.0=validated"' so their staff can easily ensure that the correct version of the pipeline is run in production. + ## Pipeline software licences Sometimes it's useful to see the software licences of the tools used in a pipeline. From 3901f2de4e4167e38e7aa05d99774ee24c42d309 Mon Sep 17 00:00:00 2001 From: Matthias Zepper Date: Fri, 26 Apr 2024 21:23:22 +0200 Subject: [PATCH 08/16] Update CHANGELOG: Minor version jump, because CLI arguments changed. --- CHANGELOG.md | 5 +++-- README.md | 2 +- tests/utils.py | 1 + 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 88ce77a3b..564860328 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # nf-core/tools: Changelog -## v2.13.2dev +## v2.14.0dev ### Template @@ -25,8 +25,9 @@ ### Download -- Replace `--tower` with `--platform`. The former will remain for backwards compatability for now but will be removed in a future release. +- Replace `--tower` with `--platform`. The former will remain for backwards compatability for now but will be removed in a future release. ([#2853](https://github.com/nf-core/tools/pull/2853)) - Better error message when GITHUB_TOKEN exists but is wrong/outdated +- New `-a` / `--additional-tag` argument to add custom tags during a pipeline download ([#2938](https://github.com/nf-core/tools/pull/2938)) ### Components diff --git a/README.md b/README.md index 65cfc21b3..b28be6efa 100644 --- a/README.md +++ b/README.md @@ -437,7 +437,7 @@ Subsequently, the `*.git` folder can be moved to it's final destination and link > [!TIP] > Also without access to Seqera Platform, pipelines downloaded with the `--platform` flag can be run if the _absolute_ path is specified: `nextflow run -r 2.5 file:/path/to/pipelinedownload.git`. Downloads in this format allow you to include multiple revisions of a pipeline in a single file, but require that the revision (e.g. `-r 2.5`) is always explicitly specified. -Facilities and those who are setting up pipelines for others to use may find the '-a' / '--additional-tag' argument helpful. It must be followed by a string in a "key=value" format and can be provided multiple times. The left-hand side must refer to a valid branch, tag or commit SHA. The right-hand side must comply with the naming conventions for Git tags and may not yet exist in the repository. The '-a' / '--additional-tag' argument allows to customize the downloaded pipeline with additional tags that can be used to select particular revisions in the Seqera Platform interface. For example, an accredited facility may opt to tag particular revisions according to their structured release management process: '-a "3.12.0=testing" -a "3.9.0=validated"' so their staff can easily ensure that the correct version of the pipeline is run in production. +Facilities and those who are setting up pipelines for others to use may find the `-a` / `--additional-tag` argument helpful. It must be followed by a string in a "key=value" format and can be provided multiple times. The left-hand side must refer to a valid branch, tag or commit SHA. The right-hand side must comply with the naming conventions for Git tags and may not yet exist in the repository. The `-a` / `--additional-tag` argument allows customizing the downloaded pipeline with additional tags that can be used to select particular revisions in the Seqera Platform interface. For example, an accredited facility may opt to tag particular revisions according to their structured release management process: `-a "3.12.0=testing" -a "3.9.0=validated"` so their staff can easily ensure that the correct version of the pipeline is run in production. ## Pipeline software licences diff --git a/tests/utils.py b/tests/utils.py index 4635c53b7..89c132881 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -56,6 +56,7 @@ def wrapper(*args: Any, **kwargs: Any) -> Any: return wrapper + def mock_anaconda_api_calls(rsps: responses.RequestsMock, module: str, version: str) -> None: """Mock anaconda api calls for module""" anaconda_api_url = f"https://api.anaconda.org/package/bioconda/{module}" From 00cb57eecc9c831d6f10f95ad86c83e33d840f0d Mon Sep 17 00:00:00 2001 From: Matthias Zepper Date: Fri, 26 Apr 2024 21:42:01 +0200 Subject: [PATCH 09/16] Ensure a user is configured in Git config. Required for Github Actions, but may also affect normal usage. --- nf_core/download.py | 18 +++++++++++++----- nf_core/synced_repo.py | 16 ++++++++++++++++ tests/test_download.py | 35 ++++++++++++++++++++++------------- 3 files changed, 51 insertions(+), 18 deletions(-) diff --git a/nf_core/download.py b/nf_core/download.py index 62fe08e7d..89a2e39d7 100644 --- a/nf_core/download.py +++ b/nf_core/download.py @@ -1726,21 +1726,29 @@ def tidy_tags_and_branches(self): # "Private" method to add the additional custom tags to the repository. def __add_additional_tags(self) -> None: if self.additional_tags: + self.ensure_git_user_config(f"nf-core download v{nf_core.__version__}", "core@nf-co.re") + for additional_tag in self.additional_tags: # A valid git branch or tag name can contain alphanumeric characters, underscores, hyphens, and dots. # But it must not start with a dot, hyphen or underscore and also cannot contain two consecutive dots. - if re.match(r"^\w[\w_.-]+={1}\w[\w_.-]+$",additional_tag) and '..' not in additional_tag: + if re.match(r"^\w[\w_.-]+={1}\w[\w_.-]+$", additional_tag) and ".." not in additional_tag: anchor, tag = additional_tag.split("=") if self.repo.is_valid_object(anchor) and not self.repo.is_valid_object(tag): try: - self.repo.create_tag(tag,ref=anchor,message=f"Synonynmous tag to {anchor}; added by 'nf-core download'.") - self.repo.create_head(tag,anchor) # should heads be created as well? + self.repo.create_tag( + tag, ref=anchor, message=f"Synonynmous tag to {anchor}; added by 'nf-core download'." + ) + self.repo.create_head(tag, anchor) # should heads be created as well? except (GitCommandError, InvalidGitRepositoryError) as e: log.error(f"[red]Additional tag(s) could not be applied:[/]\n{e}\n") else: - log.error(f"[red]Adding the additional tag '{tag}' to '{anchor}' failed.[/]\n Mind that '{anchor}' must be a valid git reference that resolves to a commit, while '{tag}' must not exist hitherto.") + log.error( + f"[red]Adding the additional tag '{tag}' to '{anchor}' failed.[/]\n Mind that '{anchor}' must be a valid git reference that resolves to a commit, while '{tag}' must not exist hitherto." + ) else: - log.error(f"[red]Could not apply invalid '-a' / '--additional-tag' specification[/]: '{additional_tag}'") + log.error( + f"[red]Could not apply invalid '-a' / '--additional-tag' specification[/]: '{additional_tag}'" + ) def bare_clone(self, destination): if self.repo: diff --git a/nf_core/synced_repo.py b/nf_core/synced_repo.py index 5c31e9691..5e1587bae 100644 --- a/nf_core/synced_repo.py +++ b/nf_core/synced_repo.py @@ -116,6 +116,10 @@ def __init__(self, remote_url=None, branch=None, no_pull=False, hide_progress=Fa self.remote_url = remote_url + self.repo = None + # ToDo: SyncedRepo doesn't have this method and both the ModulesRepo and + # the WorkflowRepo define their own including custom init methods. This needs + # fixing, but is beyond the scope of this PR. self.setup_local_repo(remote_url, branch, hide_progress) config_fn, repo_config = load_tools_config(self.local_repo_dir) @@ -326,6 +330,18 @@ def component_files_identical(self, component_name, base_path, commit, component self.checkout_branch() return files_identical + def ensure_git_user_config(self, default_name: str, default_email: str) -> None: + with self.repo.config_reader() as git_config: + user_name = git_config.get_value("user", "name", default=None) + user_email = git_config.get_value("user", "email", default=None) + + if not user_name or not user_email: + with self.repo.config_writer() as git_config: + if not user_name: + git_config.set_value("user", "name", default_name) + if not user_email: + git_config.set_value("user", "email", default_email) + def get_component_git_log(self, component_name, component_type, depth=None): """ Fetches the commit history the of requested module/subworkflow since a given date. The default value is diff --git a/tests/test_download.py b/tests/test_download.py index 6928a9087..9794151ab 100644 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -21,22 +21,21 @@ class DownloadTest(unittest.TestCase): - @pytest.fixture(autouse=True) def use_caplog(self, caplog): self._caplog = caplog @property - def logged_levels(self) -> list[str]: + def logged_levels(self) -> list: return [record.levelname for record in self._caplog.records] @property - def logged_messages(self) -> list[str]: + def logged_messages(self) -> list: return [record.message for record in self._caplog.records] def __contains__(self, item: str) -> bool: """Allows to check for log messages easily using the in operator inside a test: - assert 'my log message' in self + assert 'my log message' in self """ return any(record.message == item for record in self._caplog.records if self._caplog) @@ -644,15 +643,13 @@ def test_download_workflow_for_platform(self, tmp_dir, _): in download_obj.containers ) # indirect definition via $container variable. - # # Test adding custom tags to Seqera Platform download # @mock.patch("nf_core.download.DownloadWorkflow.get_singularity_images") @with_temporary_folder - def test_download_workflow_for_platform_with_custom_tags(self,_, tmp_dir): + def test_download_workflow_for_platform_with_custom_tags(self, _, tmp_dir): with self._caplog.at_level(logging.INFO): - from git.refs.head import Head from git.refs.tag import TagReference download_obj = DownloadWorkflow( @@ -661,7 +658,13 @@ def test_download_workflow_for_platform_with_custom_tags(self,_, tmp_dir): compress_type="none", platform=True, container_system=None, - additional_tags=("3.7=a.tad.outdated", "3.9=cool_revision", "3.9=invalid tag","3.14.0=not_included","What is this?"), + additional_tags=( + "3.7=a.tad.outdated", + "3.9=cool_revision", + "3.9=invalid tag", + "3.14.0=not_included", + "What is this?", + ), ) download_obj.include_configs = False # suppress prompt, because stderr.is_interactive doesn't. @@ -696,8 +699,14 @@ def test_download_workflow_for_platform_with_custom_tags(self,_, tmp_dir): workflow_repo_tags = {tag.name for tag in download_obj.workflow_repo.tags} assert len(workflow_repo_tags) == 4 # the invalid/malformed additional_tags should not have been added. - assert all(tag in workflow_repo_tags for tag in {'3.7', 'a.tad.outdated', 'cool_revision', '3.9'}) - assert not any(tag in workflow_repo_tags for tag in {'invalid tag','not_included','What is this?'}) - - assert all(log in self.logged_messages for log in {"[red]Could not apply invalid '-a' / '--additional-tag' specification[/]: '3.9=invalid tag'", "[red]Adding the additional tag 'not_included' to '3.14.0' failed.[/]\n Mind that '3.14.0' must be a valid git reference that resolves to a commit, while 'not_included' must not exist hitherto.", "[red]Could not apply invalid '-a' / '--additional-tag' specification[/]: 'What is this?'"}) - + assert all(tag in workflow_repo_tags for tag in {"3.7", "a.tad.outdated", "cool_revision", "3.9"}) + assert not any(tag in workflow_repo_tags for tag in {"invalid tag", "not_included", "What is this?"}) + + assert all( + log in self.logged_messages + for log in { + "[red]Could not apply invalid '-a' / '--additional-tag' specification[/]: '3.9=invalid tag'", + "[red]Adding the additional tag 'not_included' to '3.14.0' failed.[/]\n Mind that '3.14.0' must be a valid git reference that resolves to a commit, while 'not_included' must not exist hitherto.", + "[red]Could not apply invalid '-a' / '--additional-tag' specification[/]: 'What is this?'", + } + ) From e089676df548596dc6162115759e47e0e6cbf064 Mon Sep 17 00:00:00 2001 From: Matthias Zepper Date: Mon, 29 Apr 2024 16:23:37 +0200 Subject: [PATCH 10/16] Catch exceptions if section is missing in the Git config. --- nf_core/synced_repo.py | 10 +++++++--- tests/test_download.py | 22 ++++++++++++++++++++-- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/nf_core/synced_repo.py b/nf_core/synced_repo.py index 5e1587bae..f72309780 100644 --- a/nf_core/synced_repo.py +++ b/nf_core/synced_repo.py @@ -2,6 +2,7 @@ import logging import os import shutil +from configparser import NoOptionError, NoSectionError from pathlib import Path from typing import Dict @@ -331,9 +332,12 @@ def component_files_identical(self, component_name, base_path, commit, component return files_identical def ensure_git_user_config(self, default_name: str, default_email: str) -> None: - with self.repo.config_reader() as git_config: - user_name = git_config.get_value("user", "name", default=None) - user_email = git_config.get_value("user", "email", default=None) + try: + with self.repo.config_reader() as git_config: + user_name = git_config.get_value("user", "name", default=None) + user_email = git_config.get_value("user", "email", default=None) + except (NoOptionError, NoSectionError): + user_name = user_email = None if not user_name or not user_email: with self.repo.config_writer() as git_config: diff --git a/tests/test_download.py b/tests/test_download.py index 9794151ab..8213b980d 100644 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -7,6 +7,7 @@ import tempfile import unittest from pathlib import Path +from typing import List from unittest import mock import pytest @@ -26,11 +27,11 @@ def use_caplog(self, caplog): self._caplog = caplog @property - def logged_levels(self) -> list: + def logged_levels(self) -> List[str]: return [record.levelname for record in self._caplog.records] @property - def logged_messages(self) -> list: + def logged_messages(self) -> List[str]: return [record.message for record in self._caplog.records] def __contains__(self, item: str) -> bool: @@ -689,6 +690,7 @@ def test_download_workflow_for_platform_with_custom_tags(self, _, tmp_dir): assert download_obj.workflow_repo assert isinstance(download_obj.workflow_repo, WorkflowRepo) assert issubclass(type(download_obj.workflow_repo), SyncedRepo) + assert "Locally cached repository: nf-core/rnaseq, revisions 3.7, 3.9" in repr(download_obj.workflow_repo) # assert that every additional tag has been passed on to the WorkflowRepo instance assert download_obj.additional_tags == download_obj.workflow_repo.additional_tags @@ -710,3 +712,19 @@ def test_download_workflow_for_platform_with_custom_tags(self, _, tmp_dir): "[red]Could not apply invalid '-a' / '--additional-tag' specification[/]: 'What is this?'", } ) + + # + # Test adding a single custom tags to Seqera Platform download so CodeCov is happy. + # + @mock.patch("nf_core.download.DownloadWorkflow.get_singularity_images") + @with_temporary_folder + def test_download_workflow_for_platform_with_one_custom_tag(self, _, tmp_dir): + download_obj = DownloadWorkflow( + pipeline="nf-core/rnaseq", + revision=("3.9"), + compress_type="none", + platform=True, + container_system=None, + additional_tags=("3.9=cool_revision",), + ) + assert isinstance(download_obj.additional_tags, list) and len(download_obj.additional_tags) == 1 From 6688079dc4bf891847a6d99c30b9fcf2a6d4a038 Mon Sep 17 00:00:00 2001 From: Matthias Zepper <6963520+MatthiasZepper@users.noreply.github.com> Date: Fri, 3 May 2024 18:37:01 +0200 Subject: [PATCH 11/16] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Matthias Hörtenhuber --- README.md | 2 +- nf_core/download.py | 2 +- nf_core/synced_repo.py | 4 ++-- tests/test_download.py | 3 --- 4 files changed, 4 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index b28be6efa..b63514b44 100644 --- a/README.md +++ b/README.md @@ -437,7 +437,7 @@ Subsequently, the `*.git` folder can be moved to it's final destination and link > [!TIP] > Also without access to Seqera Platform, pipelines downloaded with the `--platform` flag can be run if the _absolute_ path is specified: `nextflow run -r 2.5 file:/path/to/pipelinedownload.git`. Downloads in this format allow you to include multiple revisions of a pipeline in a single file, but require that the revision (e.g. `-r 2.5`) is always explicitly specified. -Facilities and those who are setting up pipelines for others to use may find the `-a` / `--additional-tag` argument helpful. It must be followed by a string in a "key=value" format and can be provided multiple times. The left-hand side must refer to a valid branch, tag or commit SHA. The right-hand side must comply with the naming conventions for Git tags and may not yet exist in the repository. The `-a` / `--additional-tag` argument allows customizing the downloaded pipeline with additional tags that can be used to select particular revisions in the Seqera Platform interface. For example, an accredited facility may opt to tag particular revisions according to their structured release management process: `-a "3.12.0=testing" -a "3.9.0=validated"` so their staff can easily ensure that the correct version of the pipeline is run in production. +Facilities and those who are setting up pipelines for others to use may find the `-a` / `--additional-tag` argument helpful. It must be followed by a string in a `key=value` format and can be provided multiple times. The `key` must refer to a valid branch, tag or commit SHA. The right-hand side must comply with the naming conventions for Git tags and may not yet exist in the repository. The `-a` / `--additional-tag` argument allows customizing the downloaded pipeline with additional tags that can be used to select particular revisions in the Seqera Platform interface. For example, an accredited facility may opt to tag particular revisions according to their structured release management process: `-a "3.12.0=testing" -a "3.9.0=validated"` so their staff can easily ensure that the correct version of the pipeline is run in production. ## Pipeline software licences diff --git a/nf_core/download.py b/nf_core/download.py index 89a2e39d7..26e48958f 100644 --- a/nf_core/download.py +++ b/nf_core/download.py @@ -300,7 +300,7 @@ def download_workflow_platform(self, location=None): remote_url=f"https://github.com/{self.pipeline}.git", revision=self.revision if self.revision else None, commit=self.wf_sha.values() if bool(self.wf_sha) else None, - additional_tags=self.additional_tags if self.additional_tags else None, + additional_tags=self.additional_tags, location=(location if location else None), # manual location is required for the tests to work in_cache=False, ) diff --git a/nf_core/synced_repo.py b/nf_core/synced_repo.py index f72309780..d4f302254 100644 --- a/nf_core/synced_repo.py +++ b/nf_core/synced_repo.py @@ -118,9 +118,9 @@ def __init__(self, remote_url=None, branch=None, no_pull=False, hide_progress=Fa self.remote_url = remote_url self.repo = None - # ToDo: SyncedRepo doesn't have this method and both the ModulesRepo and + # TODO: SyncedRepo doesn't have this method and both the ModulesRepo and # the WorkflowRepo define their own including custom init methods. This needs - # fixing, but is beyond the scope of this PR. + # fixing. self.setup_local_repo(remote_url, branch, hide_progress) config_fn, repo_config = load_tools_config(self.local_repo_dir) diff --git a/tests/test_download.py b/tests/test_download.py index 8213b980d..589fe6eda 100644 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -713,9 +713,6 @@ def test_download_workflow_for_platform_with_custom_tags(self, _, tmp_dir): } ) - # - # Test adding a single custom tags to Seqera Platform download so CodeCov is happy. - # @mock.patch("nf_core.download.DownloadWorkflow.get_singularity_images") @with_temporary_folder def test_download_workflow_for_platform_with_one_custom_tag(self, _, tmp_dir): From 4d2e68148f75f876b429d03206cc0af98fa822c8 Mon Sep 17 00:00:00 2001 From: Matthias Zepper Date: Fri, 3 May 2024 18:59:19 +0200 Subject: [PATCH 12/16] Update --additional-tag to --tag. --- CHANGELOG.md | 2 +- README.md | 2 +- nf_core/__main__.py | 11 +++++------ nf_core/download.py | 19 +++++++++++-------- tests/test_cli.py | 4 ++-- tests/test_download.py | 37 ++++++++++++++++++++----------------- 6 files changed, 40 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 564860328..d6218f7d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,7 @@ - Replace `--tower` with `--platform`. The former will remain for backwards compatability for now but will be removed in a future release. ([#2853](https://github.com/nf-core/tools/pull/2853)) - Better error message when GITHUB_TOKEN exists but is wrong/outdated -- New `-a` / `--additional-tag` argument to add custom tags during a pipeline download ([#2938](https://github.com/nf-core/tools/pull/2938)) +- New `--tag` argument to add custom tags during a pipeline download ([#2938](https://github.com/nf-core/tools/pull/2938)) ### Components diff --git a/README.md b/README.md index b63514b44..fa909804c 100644 --- a/README.md +++ b/README.md @@ -437,7 +437,7 @@ Subsequently, the `*.git` folder can be moved to it's final destination and link > [!TIP] > Also without access to Seqera Platform, pipelines downloaded with the `--platform` flag can be run if the _absolute_ path is specified: `nextflow run -r 2.5 file:/path/to/pipelinedownload.git`. Downloads in this format allow you to include multiple revisions of a pipeline in a single file, but require that the revision (e.g. `-r 2.5`) is always explicitly specified. -Facilities and those who are setting up pipelines for others to use may find the `-a` / `--additional-tag` argument helpful. It must be followed by a string in a `key=value` format and can be provided multiple times. The `key` must refer to a valid branch, tag or commit SHA. The right-hand side must comply with the naming conventions for Git tags and may not yet exist in the repository. The `-a` / `--additional-tag` argument allows customizing the downloaded pipeline with additional tags that can be used to select particular revisions in the Seqera Platform interface. For example, an accredited facility may opt to tag particular revisions according to their structured release management process: `-a "3.12.0=testing" -a "3.9.0=validated"` so their staff can easily ensure that the correct version of the pipeline is run in production. +Facilities and those who are setting up pipelines for others to use may find the `--tag` argument helpful. It must be followed by a string in a `key=value` format and can be provided multiple times. The `key` must refer to a valid branch, tag or commit SHA. The right-hand side must comply with the naming conventions for Git tags and may not yet exist in the repository. The `--tag` argument allows customizing the downloaded pipeline with additional tags that can be used to select particular revisions in the Seqera Platform interface. For example, an accredited facility may opt to tag particular revisions according to their structured release management process: `--tag "3.12.0=testing" --tag "3.9.0=validated"` so their staff can easily ensure that the correct version of the pipeline is run in production. ## Pipeline software licences diff --git a/nf_core/__main__.py b/nf_core/__main__.py index f3212200d..ead74aa67 100644 --- a/nf_core/__main__.py +++ b/nf_core/__main__.py @@ -368,6 +368,7 @@ def create_params_file(pipeline, revision, output, force, show_hidden): @click.option("-f", "--force", is_flag=True, default=False, help="Overwrite existing files") # TODO: Remove this in a future release. Deprecated in March 2024. @click.option( + "-t", "--tower", is_flag=True, default=False, @@ -375,7 +376,6 @@ def create_params_file(pipeline, revision, output, force, show_hidden): help="Download for Seqera Platform. DEPRECATED: Please use `--platform` instead.", ) @click.option( - "-t", "--platform", is_flag=True, default=False, @@ -389,10 +389,9 @@ def create_params_file(pipeline, revision, output, force, show_hidden): help="Include configuration profiles in download. Not available with `--platform`", ) @click.option( - "-a", - "--additional-tags", + "--tag", multiple=True, - help="Add custom alias tags to `--platform` downloads. For example, '-a \"3.10=validated\"' adds the custom 'validated' tag to the 3.10 release.", + help="Add custom alias tags to `--platform` downloads. For example, `--tag \"3.10=validated\"` adds the custom 'validated' tag to the 3.10 release.", ) # -c changed to -s for consistency with other --container arguments, where it is always the first letter of the last word. # Also -c might be used instead of -d for config in a later release, but reusing params for different options in two subsequent releases might be too error-prone. @@ -436,7 +435,7 @@ def download( tower, platform, download_configuration, - additional_tags, + tag, container_system, container_library, container_cache_utilisation, @@ -459,7 +458,7 @@ def download( force, tower or platform, # True if either specified download_configuration, - additional_tags, + tag, container_system, container_library, container_cache_utilisation, diff --git a/nf_core/download.py b/nf_core/download.py index 26e48958f..5edba513f 100644 --- a/nf_core/download.py +++ b/nf_core/download.py @@ -94,7 +94,7 @@ class DownloadWorkflow: force (bool): Flag to force download even if files already exist (overwrite existing files). Defaults to False. platform (bool): Flag to customize the download for Seqera Platform (convert to git bare repo). Defaults to False. download_configuration (str): Download the configuration files from nf-core/configs. Defaults to None. - additional_tags (List[str]): Specify additional tags to add to the downloaded pipeline. Defaults to None. + tag (List[str]): Specify additional tags to add to the downloaded pipeline. Defaults to None. container_system (str): The container system to use (e.g., "singularity"). Defaults to None. container_library (List[str]): The container libraries (registries) to use. Defaults to None. container_cache_utilisation (str): If a local or remote cache of already existing container images should be considered. Defaults to None. @@ -1736,19 +1736,22 @@ def __add_additional_tags(self) -> None: if self.repo.is_valid_object(anchor) and not self.repo.is_valid_object(tag): try: self.repo.create_tag( - tag, ref=anchor, message=f"Synonynmous tag to {anchor}; added by 'nf-core download'." + tag, ref=anchor, message=f"Synonynmous tag to {anchor}; added by `nf-core download`." ) self.repo.create_head(tag, anchor) # should heads be created as well? except (GitCommandError, InvalidGitRepositoryError) as e: log.error(f"[red]Additional tag(s) could not be applied:[/]\n{e}\n") else: - log.error( - f"[red]Adding the additional tag '{tag}' to '{anchor}' failed.[/]\n Mind that '{anchor}' must be a valid git reference that resolves to a commit, while '{tag}' must not exist hitherto." - ) + if not self.repo.is_valid_object(anchor): + log.error( + f"[red]Adding tag '{tag}' to '{anchor}' failed.[/]\n Mind that '{anchor}' must be a valid git reference that resolves to a commit." + ) + if self.repo.is_valid_object(tag): + log.error( + f"[red]Adding tag '{tag}' to '{anchor}' failed.[/]\n Mind that '{tag}' must not exist hitherto." + ) else: - log.error( - f"[red]Could not apply invalid '-a' / '--additional-tag' specification[/]: '{additional_tag}'" - ) + log.error(f"[red]Could not apply invalid `--tag` specification[/]: '{additional_tag}'") def bare_clone(self, destination): if self.repo: diff --git a/tests/test_cli.py b/tests/test_cli.py index 826008f03..5df0a3241 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -168,7 +168,7 @@ def test_cli_download(self, mock_dl): "force": None, "platform": None, "download-configuration": None, - "additional-tags": "3.12=testing", + "tag": "3.12=testing", "container-system": "singularity", "container-library": "quay.io", "container-cache-utilisation": "copy", @@ -189,7 +189,7 @@ def test_cli_download(self, mock_dl): "force" in params, "platform" in params, "download-configuration" in params, - (params["additional-tags"],), + (params["tag"],), params["container-system"], (params["container-library"],), params["container-cache-utilisation"], diff --git a/tests/test_download.py b/tests/test_download.py index 589fe6eda..3e0f11d57 100644 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -645,7 +645,23 @@ def test_download_workflow_for_platform(self, tmp_dir, _): ) # indirect definition via $container variable. # - # Test adding custom tags to Seqera Platform download + # Brief test adding a single custom tag to Seqera Platform download + # + @mock.patch("nf_core.download.DownloadWorkflow.get_singularity_images") + @with_temporary_folder + def test_download_workflow_for_platform_with_one_custom_tag(self, _, tmp_dir): + download_obj = DownloadWorkflow( + pipeline="nf-core/rnaseq", + revision=("3.9"), + compress_type="none", + platform=True, + container_system=None, + additional_tags=("3.9=cool_revision",), + ) + assert isinstance(download_obj.additional_tags, list) and len(download_obj.additional_tags) == 1 + + # + # Test adding custom tags to Seqera Platform download (full test) # @mock.patch("nf_core.download.DownloadWorkflow.get_singularity_images") @with_temporary_folder @@ -707,21 +723,8 @@ def test_download_workflow_for_platform_with_custom_tags(self, _, tmp_dir): assert all( log in self.logged_messages for log in { - "[red]Could not apply invalid '-a' / '--additional-tag' specification[/]: '3.9=invalid tag'", - "[red]Adding the additional tag 'not_included' to '3.14.0' failed.[/]\n Mind that '3.14.0' must be a valid git reference that resolves to a commit, while 'not_included' must not exist hitherto.", - "[red]Could not apply invalid '-a' / '--additional-tag' specification[/]: 'What is this?'", + "[red]Could not apply invalid `--tag` specification[/]: '3.9=invalid tag'", + "[red]Adding tag 'not_included' to '3.14.0' failed.[/]\n Mind that '3.14.0' must be a valid git reference that resolves to a commit.", + "[red]Could not apply invalid `--tag` specification[/]: 'What is this?'", } ) - - @mock.patch("nf_core.download.DownloadWorkflow.get_singularity_images") - @with_temporary_folder - def test_download_workflow_for_platform_with_one_custom_tag(self, _, tmp_dir): - download_obj = DownloadWorkflow( - pipeline="nf-core/rnaseq", - revision=("3.9"), - compress_type="none", - platform=True, - container_system=None, - additional_tags=("3.9=cool_revision",), - ) - assert isinstance(download_obj.additional_tags, list) and len(download_obj.additional_tags) == 1 From 0b8e4adf5cce24b8b55859ac8e467f2c2172f796 Mon Sep 17 00:00:00 2001 From: Matthias Zepper Date: Mon, 6 May 2024 15:30:07 +0200 Subject: [PATCH 13/16] Change the e-mail from the nf-core bot e-mail to a sinkhole address. --- nf_core/download.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/nf_core/download.py b/nf_core/download.py index 5edba513f..ee786325f 100644 --- a/nf_core/download.py +++ b/nf_core/download.py @@ -1726,7 +1726,11 @@ def tidy_tags_and_branches(self): # "Private" method to add the additional custom tags to the repository. def __add_additional_tags(self) -> None: if self.additional_tags: - self.ensure_git_user_config(f"nf-core download v{nf_core.__version__}", "core@nf-co.re") + # example.com is reserved by the Internet Assigned Numbers Authority (IANA) as special-use domain names for documentation purposes. + # Although "dev-null" is a syntactically-valid local-part that is equally valid for delivery, + # and only the receiving MTA can decide whether to accept it, it is to my best knowledge configured with + # a Postfix discard mail delivery agent (https://www.postfix.org/discard.8.html), so incoming mails should be sinkholed. + self.ensure_git_user_config(f"nf-core download v{nf_core.__version__}", "dev-null@example.com") for additional_tag in self.additional_tags: # A valid git branch or tag name can contain alphanumeric characters, underscores, hyphens, and dots. From 3277b7cdcfcaf1c4d21ec70988c6029d56560125 Mon Sep 17 00:00:00 2001 From: Matthias Zepper <6963520+MatthiasZepper@users.noreply.github.com> Date: Mon, 6 May 2024 17:34:53 +0200 Subject: [PATCH 14/16] Rearranging the sentences in README. Thanks @mashehu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Matthias Hörtenhuber --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index fa909804c..522285408 100644 --- a/README.md +++ b/README.md @@ -437,7 +437,8 @@ Subsequently, the `*.git` folder can be moved to it's final destination and link > [!TIP] > Also without access to Seqera Platform, pipelines downloaded with the `--platform` flag can be run if the _absolute_ path is specified: `nextflow run -r 2.5 file:/path/to/pipelinedownload.git`. Downloads in this format allow you to include multiple revisions of a pipeline in a single file, but require that the revision (e.g. `-r 2.5`) is always explicitly specified. -Facilities and those who are setting up pipelines for others to use may find the `--tag` argument helpful. It must be followed by a string in a `key=value` format and can be provided multiple times. The `key` must refer to a valid branch, tag or commit SHA. The right-hand side must comply with the naming conventions for Git tags and may not yet exist in the repository. The `--tag` argument allows customizing the downloaded pipeline with additional tags that can be used to select particular revisions in the Seqera Platform interface. For example, an accredited facility may opt to tag particular revisions according to their structured release management process: `--tag "3.12.0=testing" --tag "3.9.0=validated"` so their staff can easily ensure that the correct version of the pipeline is run in production. +Facilities and those who are setting up pipelines for others to use may find the `--tag` argument helpful. It allows customizing the downloaded pipeline with additional tags that can be used to select particular revisions in the Seqera Platform interface. For example, an accredited facility may opt to tag particular revisions according to their structured release management process: `--tag "3.12.0=testing" --tag "3.9.0=validated"` so their staff can easily ensure that the correct version of the pipeline is run in production. +The `--tag` argument must be followed by a string in a `key=value` format and can be provided multiple times. The `key` must refer to a valid branch, tag or commit SHA. The right-hand side must comply with the naming conventions for Git tags and may not yet exist in the repository. ## Pipeline software licences From 1e632c2d82239f9053af64b3644d7fdb829576bb Mon Sep 17 00:00:00 2001 From: Matthias Zepper Date: Tue, 7 May 2024 07:26:37 +0200 Subject: [PATCH 15/16] Add deprecation warning for -t / --tower flag. --- nf_core/__main__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/nf_core/__main__.py b/nf_core/__main__.py index ead74aa67..39a1afbf6 100644 --- a/nf_core/__main__.py +++ b/nf_core/__main__.py @@ -450,6 +450,9 @@ def download( """ from nf_core.download import DownloadWorkflow + if tower: + log.warning("[red]The `-t` / `--tower` flag is deprecated. Please use `--platform` instead.[/]") + dl = DownloadWorkflow( pipeline, revision, From c95880f25ec5e62a822daa8a26479b90f0c7056e Mon Sep 17 00:00:00 2001 From: Matthias Zepper Date: Tue, 7 May 2024 07:44:47 +0200 Subject: [PATCH 16/16] Omit creating heads for additional tags due to UI bug in Platform. --- nf_core/download.py | 1 - 1 file changed, 1 deletion(-) diff --git a/nf_core/download.py b/nf_core/download.py index ee786325f..f5ab3a0f5 100644 --- a/nf_core/download.py +++ b/nf_core/download.py @@ -1742,7 +1742,6 @@ def __add_additional_tags(self) -> None: self.repo.create_tag( tag, ref=anchor, message=f"Synonynmous tag to {anchor}; added by `nf-core download`." ) - self.repo.create_head(tag, anchor) # should heads be created as well? except (GitCommandError, InvalidGitRepositoryError) as e: log.error(f"[red]Additional tag(s) could not be applied:[/]\n{e}\n") else: