+
Skip to content

Include the centralized nf-core configs also in offline mode, if a local copy is available. #3491

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

Merged

Conversation

MatthiasZepper
Copy link
Member

@MatthiasZepper MatthiasZepper commented Mar 14, 2025

As discussed in #3430 it would be desirable to have the option to use the centralized configs also on offline HPCs, if they are available as a local copy. This PR implements this change as suggested by @bentsherman:

Rationale:

  • The new config parser does not allow for try, if or def on a top-level, so the old try notation needed updating.
  • Normally, we include the nf-core custom config from GitHub, the custom_config_base defaults to the nf-core/configs repository.
  • If an execution environment does not have access to the internet, the pipeline run would, however, fail because the custom config is not available.
  • Therefore, we do not want to include the custom config from GitHub if the NXF_OFFLINE environment variable is set, yet allow for a local path.
  • The institutional configs should be included even in offline mode, if they are locally available.

Additionally, it fixes the downstream issues that arose by this change:

  • nf-core pipelines lint had to be significantly changed to accommodate to the fact that we now have multiple old versions of that code.
  • nf_core/pipeline-template/.github/workflows/config_tests.yml is a new GitHub Action to test, if a pipeline actually has the institutional configs available. It is run on pull requests to main, such that any issues should be caught prior to a pipeline release.
  • nf-core pipelines create was adapted so that the aforementioned GitHub Action is not included in a customized template when opting out of the centralized nf-core profiles.

PR checklist

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md is updated
  • If you've fixed a bug or added code that should be tested, add tests!
  • Documentation in docs is updated

@MatthiasZepper MatthiasZepper force-pushed the local_config_inclusion_when_offline branch 8 times, most recently from 21612fb to 42e8fda Compare March 18, 2025 16:44
Copy link

codecov bot commented Mar 18, 2025

Codecov Report

Attention: Patch coverage is 70.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 76.91%. Comparing base (49d0608) to head (01d911e).
Report is 14 commits behind head on dev.

Files with missing lines Patch % Lines
nf_core/pipelines/lint/nextflow_config.py 70.00% 3 Missing ⚠️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@MatthiasZepper
Copy link
Member Author

Does somebody have any pointers on the CI failures?

During my manual tests, it seemed to work fine:

pip3 install --upgrade --force-reinstall git+https://github.com/MatthiasZepper/tools.git@local_config_inclusion_when_offline

@mirpedrol
Copy link
Member

It's due to the Nextflow edge version 😞 we have seen this on pipelines too. Nextflow team is working on a fix

@MatthiasZepper
Copy link
Member Author

Ok, then we will wait. But otherwise, I think this is ready for review.

@MatthiasZepper MatthiasZepper marked this pull request as ready for review March 18, 2025 17:20
@MatthiasZepper MatthiasZepper force-pushed the local_config_inclusion_when_offline branch 2 times, most recently from 9df5d67 to 65b98b6 Compare March 24, 2025 19:55
Copy link
Contributor

@Aratz Aratz left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for addressing this!

@MatthiasZepper MatthiasZepper force-pushed the local_config_inclusion_when_offline branch from 65b98b6 to 1ede28b Compare March 25, 2025 10:21
@MatthiasZepper MatthiasZepper added this to the 3.3.0 milestone Mar 31, 2025
@MatthiasZepper MatthiasZepper force-pushed the local_config_inclusion_when_offline branch from 34acde8 to a81314d Compare April 2, 2025 13:47
@MatthiasZepper MatthiasZepper requested a review from mirpedrol April 2, 2025 14:15
@MatthiasZepper MatthiasZepper force-pushed the local_config_inclusion_when_offline branch from a1c71b0 to 01d911e Compare April 3, 2025 10:02
@MatthiasZepper MatthiasZepper merged commit ae7760b into nf-core:dev Apr 3, 2025
95 checks passed
@MatthiasZepper MatthiasZepper deleted the local_config_inclusion_when_offline branch April 3, 2025 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants
点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载