-
Notifications
You must be signed in to change notification settings - Fork 190
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
Template: Add a Github Action workflow to test a successful download of the pipeline #2618
Merged
MatthiasZepper
merged 5 commits into
nf-core:dev
from
MatthiasZepper:GithubAction-DownloadTest
Jan 8, 2024
Merged
Changes from all commits
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 d4f3dc2
Only run on self-hosted runners due to space constraints in GitHub ho…
MatthiasZepper f93b924
Since we now limit it to PRs to master, updating a PR branch should b…
MatthiasZepper 9fb5498
Apply suggestions from code review
MatthiasZepper 40eb38d
Merge branch 'dev' into GithubAction-DownloadTest
MatthiasZepper File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
68 changes: 68 additions & 0 deletions
68
nf_core/pipeline-template/.github/workflows/download_pipeline.yml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
name: Test successful pipeline download with 'nf-core download' | ||
|
||
# Run the workflow when: | ||
# - dispatched manually | ||
# - when a PR is opened or reopened to master branch | ||
# - the head branch of the pull request is updated, i.e. if fixes for a release are pushed last minute to dev. | ||
|
||
on: | ||
workflow_dispatch: | ||
pull_request: | ||
types: | ||
- opened | ||
branches: | ||
- master | ||
pull_request_target: | ||
branches: | ||
- master | ||
|
||
env: | ||
NXF_ANSI_LOG: false | ||
|
||
jobs: | ||
download: | ||
runs-on: ["self-hosted"] | ||
steps: | ||
- name: Install Nextflow | ||
uses: nf-core/setup-nextflow@v1 | ||
|
||
- uses: actions/setup-python@v5 | ||
with: | ||
python-version: "3.11" | ||
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 | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 theNXF_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!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point 👍🏻
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 👍🏻
There was a problem hiding this comment.
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.