Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Tower UI no longer shows revisions for downloaded pipelines. #2308

Merged
merged 9 commits into from
Jun 12, 2023
65 changes: 52 additions & 13 deletions nf_core/download.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import tarfile
import textwrap
from datetime import datetime
from distutils.version import StrictVersion
from zipfile import ZipFile

import git
Expand All @@ -27,7 +28,11 @@
import nf_core.list
import nf_core.utils
from nf_core.synced_repo import RemoteProgressbar, SyncedRepo
from nf_core.utils import NFCORE_CACHE_DIR, NFCORE_DIR
from nf_core.utils import (
NFCORE_CACHE_DIR,
NFCORE_DIR,
SingularityCacheFilePathValidator,
)

log = logging.getLogger(__name__)
stderr = rich.console.Console(
Expand Down Expand Up @@ -157,7 +162,7 @@ def download_workflow(self):
sys.exit(1)

summary_log = [
f"Pipeline revision: '{', '.join(self.revision) if len(self.revision) < 5 else self.revision[0]+',...,['+str(len(self.revision)-2)+' more revisions],...,'+self.revision[-1]}'",
f"Pipeline revision: '{', '.join(self.revision) if len(self.revision) < 5 else self.revision[0]+',['+str(len(self.revision)-2)+' more revisions],'+self.revision[-1]}'",
f"Pull containers: '{self.container}'",
]
if self.container == "singularity" and os.environ.get("NXF_SINGULARITY_CACHEDIR") is not None:
Expand Down Expand Up @@ -493,8 +498,8 @@ def prompt_singularity_cachedir_remote(self):
cachedir_index = None
while cachedir_index is None:
prompt_cachedir_index = questionary.path(
"Specify a list of the remote images already present in the remote system :",
file_filter="*.txt",
"Specify a list of the container images that are already present on the remote system:",
validate=SingularityCacheFilePathValidator,
style=nf_core.utils.nfcore_question_style,
).unsafe_ask()
cachedir_index = os.path.abspath(os.path.expanduser(prompt_cachedir_index))
Expand Down Expand Up @@ -531,7 +536,8 @@ def read_remote_containers(self):
except (FileNotFoundError, LookupError) as e:
log.error(f"[red]Issue with reading the specified remote $NXF_SINGULARITY_CACHE index:[/]\n{e}\n")
if stderr.is_interactive and rich.prompt.Confirm.ask(f"[blue]Specify a new index file and try again?"):
self.prompt_singularity_cachedir_remote(retry=True)
self.singularity_cache_index = None # reset chosen path to index file.
self.prompt_singularity_cachedir_remote()
else:
log.info("Proceeding without consideration of the remote $NXF_SINGULARITY_CACHE index.")
self.singularity_cache_index = None
Expand Down Expand Up @@ -731,7 +737,7 @@ def find_container_images(self, workflow_directory):
r"(?<=container)[^\${}]+\${([^{}]+)}(?![^{]*})", contents
)

if bool(container_definition) & bool(container_definition.group(1)):
if bool(container_definition) and bool(container_definition.group(1)):
pattern = re.escape(container_definition.group(1))
# extract the quoted string(s) following the variable assignment
container_names = re.findall(r"%s\s*=\s*[\"\']([^\"\']+)[\"\']" % pattern, contents)
Expand Down Expand Up @@ -1048,9 +1054,18 @@ def singularity_pull_image(self, container, out_path, cache_path, progress):
if lines:
# something went wrong with the container retrieval
if any("FATAL: " in line for line in lines):
log.info("Singularity container retrieval failed with the following error:")
log.info("".join(lines))
raise FileNotFoundError(f'The container "{container}" is unavailable.\n{"".join(lines)}')
log.error("Singularity container retrieval failed with the following error:")
MatthiasZepper marked this conversation as resolved.
Show resolved Hide resolved
log.error("".join(lines))
if stderr.is_interactive and rich.prompt.Confirm.ask(
f"[blue]Continue downloading the workflow nonetheless?"
MatthiasZepper marked this conversation as resolved.
Show resolved Hide resolved
):
log.error(
f'Skipped download of "{container}".\n Please troubleshoot command "{" ".join(singularity_command)}" on an individual basis.'
MatthiasZepper marked this conversation as resolved.
Show resolved Hide resolved
)
progress.remove_task(task)
return
else:
raise FileNotFoundError(f'Downloading image "{container}" unfortunately failed.')

# Copy cached download if we are using the cache
if cache_path:
Expand Down Expand Up @@ -1252,8 +1267,8 @@ def tidy_tags_and_branches(self):
desired_revisions = set(self.revision)

# determine what needs pruning
tags_to_remove = {tag for tag in self.repo.tags if tag.name not in desired_revisions}
heads_to_remove = {head for head in self.repo.heads if head.name not in desired_revisions}
tags_to_remove = {tag for tag in self.repo.tags if tag.name not in desired_revisions.union({"latest"})}
heads_to_remove = {head for head in self.repo.heads if head.name not in desired_revisions.union({"latest"})}

try:
# delete unwanted tags from repository
Expand All @@ -1268,9 +1283,33 @@ def tidy_tags_and_branches(self):
for head in heads_to_remove:
self.repo.delete_head(head)

# ensure all desired branches are available
# ensure all desired revisions/branches are available
for revision in desired_revisions:
self.checkout(revision)
if not self.repo.is_valid_object(revision):
self.checkout(revision)

# no branch exists, but one is required for Tower's UI to display revisions correctly). Thus, "latest" will be created.
if not bool(self.repo.heads):
if self.repo.is_valid_object("latest"):
# "latest" exists as tag but not as branch
self.repo.create_head("latest", "latest") # create a new head for latest
self.checkout("latest")
else:
# "latest" is neither a branch (no heads exist) nor a tag, since is_valid_object() returned False.
# try to determine highest contained version number from desired revisions, to which latest will be pinned.
try:
latest = sorted(desired_revisions, key=StrictVersion, reverse=True)[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

why don't you use looseversion here?

Copy link
Member Author

@MatthiasZepper MatthiasZepper Jun 9, 2023

Choose a reason for hiding this comment

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

distutils module is unfortunately deprecated. Will therefore rewrite and use neither StrictVersion nor LooseVersion. Took a simplified Regex derived from the one in PEP386.

except (
ValueError
): # sorting might fail, because desired revisions may have names that do not comply with semantic version requirements.
# retry sorting using first letters only, this should also sort 3.10 before 3.5
latest = sorted(desired_revisions, key=lambda x: str(x)[0:4])[0]
finally:
self.repo.create_head("latest", latest)
self.checkout(latest)
if self.repo.head.is_detached:
self.repo.head.reset(index=True, working_tree=True)

self.heads = self.repo.heads

# get all tags and available remote_branches
Expand Down
17 changes: 17 additions & 0 deletions nf_core/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -880,6 +880,23 @@ def prompt_pipeline_release_branch(wf_releases, wf_branches, multiple=False):
)


class SingularityCacheFilePathValidator(questionary.Validator):
"""
Validator for file path specified as --singularity-cache-index argument in nf-core download
"""

def validate(self, value):
if len(value.text):
if os.path.isfile(value.text):
return True
else:
raise questionary.ValidationError(
message="Invalid remote cache index file", cursor_position=len(value.text)
)
else:
return True


def get_repo_releases_branches(pipeline, wfs):
"""Fetches details of a nf-core workflow to download.

Expand Down
8 changes: 7 additions & 1 deletion tests/test_download.py
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,14 @@ def test_download_workflow_for_tower(self, tmp_dir):
assert download_obj.workflow_repo
assert isinstance(download_obj.workflow_repo, WorkflowRepo)
assert issubclass(type(download_obj.workflow_repo), SyncedRepo)

# corroborate that the other revisions are inaccessible to the user.
assert len(download_obj.workflow_repo.tags) == len(download_obj.revision)
all_tags = {tag.name for tag in download_obj.workflow_repo.tags}
all_heads = {head.name for head in download_obj.workflow_repo.heads}

assert set(download_obj.revision) == all_tags
# assert that the download has a "latest" branch.
assert "latest" in all_heads

# download_obj.download_workflow_tower(location=tmp_dir) will run container image detection for all requested revisions
assert isinstance(download_obj.containers, list) and len(download_obj.containers) == 33
Expand Down