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.

Sign up for GitHub

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

Jump to bottom

Template: Add a Github Action workflow to test a successful download of the pipeline #2618

Merged
merged 5 commits into from
Jan 8, 2024
Merged

Template: Add a Github Action workflow to test a successful download of the pipeline #2618

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6ea4d62
Add a Github Action Workflow to the pipeline template that tests a su…
MatthiasZepper Dec 20, 2023
d4f3dc2
Only run on self-hosted runners due to space constraints in GitHub ho…
MatthiasZepper Jan 4, 2024
f93b924
Since we now limit it to PRs to master, updating a PR branch should b…
MatthiasZepper Jan 4, 2024
9fb5498
Apply suggestions from code review
MatthiasZepper Jan 8, 2024
40eb38d
Merge branch 'dev' into GithubAction-DownloadTest
MatthiasZepper Jan 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

### Template

- Add a Github Action Workflow to the pipeline template that tests a successful download with 'nf-core download' ([#2618](https://github.com/nf-core/tools/pull/2618))

### Download

### Linting
Expand Down
70 changes: 70 additions & 0 deletions nf_core/pipeline-template/.github/workflows/download_pipeline.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
name: Test successful pipeline download with 'nf-core download'

# Run the workflow when:
# - dispatched manually
# - when a PR is opened or reopened to dev or master branches
MatthiasZepper marked this conversation as resolved.
Show resolved Hide resolved
# - the head branch of the pull request is updated. (Do we need this?)

on:
workflow_dispatch:
pull_request:
types:
- opened
branches:
- master
- dev
MatthiasZepper marked this conversation as resolved.
Show resolved Hide resolved
#pull_request_target:
# branches:
# - master
# - dev

env:
NXF_ANSI_LOG: false

jobs:
download:
runs-on: ubuntu-latest
steps:
- name: Install Nextflow
uses: nf-core/setup-nextflow@v1

- uses: actions/setup-python@v5
with:
python-version: "3.10"
MatthiasZepper marked this conversation as resolved.
Show resolved Hide resolved
architecture: "x64"
- uses: eWaterCycle/setup-singularity@v7
with:
singularity-version: 3.8.3

- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install git+https://github.com/nf-core/tools.git@dev

- name: Get the repository name and current branch set as environment variable
run: |
echo "REPO_LOWERCASE=${GITHUB_REPOSITORY,,}" >> ${GITHUB_ENV}
echo "REPOTITLE_LOWERCASE=$(basename ${GITHUB_REPOSITORY,,})" >> ${GITHUB_ENV}
echo "REPO_BRANCH=${GITHUB_REF#refs/heads/}" >> ${GITHUB_ENV}

- name: Download the pipeline
env:
NXF_SINGULARITY_CACHEDIR: ./
run: |
nf-core download ${{ env.REPO_LOWERCASE }} \
--revision ${{ env.REPO_BRANCH }} \
--outdir ./${{ env.REPOTITLE_LOWERCASE }} \
--compress "none" \
--container-system 'singularity' \
--container-library "quay.io" -l "docker.io" -l "ghcr.io" \
--container-cache-utilisation 'amend' \
--download-configuration

- name: Inspect download
run: tree ./${{ env.REPOTITLE_LOWERCASE }}

- name: Run the downloaded pipeline
env:
NXF_SINGULARITY_CACHEDIR: ./
NXF_SINGULARITY_HOME_MOUNT: true
run: nextflow run ./${{ env.REPOTITLE_LOWERCASE }}/$( sed 's/\W/_/g' <<< ${{ env.REPO_BRANCH }}) -stub -profile test,singularity --outdir ./results
Copy link
Member

Choose a reason for hiding this comment

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

What's to stop Nextflow from downloading any missing singularity containers during run time here? eg. if the singularity image has been downloaded with an incorrect filename?

Tagging @mirpedrol @mashehu

Copy link
Member Author

Choose a reason for hiding this comment

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

That is a valid point, although the case of an incorrect filename should be caught by the respective tests in the tools repo.

Do you think that setting NXF_OFFLINE would help? Or should we count the files in the NXF_SINGULARITY_CACHEDIR before and after the pipeline run to corroborate that they are still the name number?

Generally, though, I am already very happy that there is now a download test in the first place, because we had multiple new releases for which nf-core download unexpectedly raised exceptions because some odd declaration slipped through the module linting and review. (Such things like arbitrarily mixing single and double quotes). If we can avoid publishing some crooked module as part of a pipeline release like this, I am already very happy!

Copy link
Member

Choose a reason for hiding this comment

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

That is a valid point, although the case of an incorrect filename should be caught by the respective tests in the tools repo.

Good point 👍🏻

Do you think that setting NXF_OFFLINE would help?

We were chatting about that in the meeting where I added the comment. I'm not confident that it would 👀 😅 We were playing with the idea of cutting off networking access on the custom runner and stuff, but I think it's almost certainly overkill.

No need to do anything here, as you say - invalid container names is something that we should be testing in other tests. Testing for syntax errors / weirdness here is a good addition as it is 👍🏻

Copy link
Member Author

@MatthiasZepper MatthiasZepper Jan 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

If you cut networking access entirely, then the most likely test failure will be some issue with the plugins rather than the actual test subject, namely container images. 😅

I already searched a bit and there seems to be ways to block network traffic selectively, but only on a per job and not on a per-step basis. So if one disallows the registries, the containers would not download successfully beforehand either.

Loading