Download pipelines with authenticated GH API calls #3607
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
|
Oops wrong base branch. Fixed. |
|
Done. Test failures (most likely) unrelated or at least I have no idea what they mean. |
There is a problem mapping your CLI arguments to the parameters of the Apart from that, please mind that there is a major refactor of Downloads ongoing (#3634). Preferably, all new contributions would already be based on and point to this new structure. |
|
How does this fit with the pipeline downloads refactoring @MatthiasZepper @jpfeuffer? |
|
Good question. Who knows more about the refactor? |
|
Yes, I checked. And the mechanism for a download of the repo data/files is still the same. |
|
And well there needs to be a little fix for the CLI apparently. Note that I just added the CLI option because I still wanted to allow downloads from the Zip URLs because they will never be rate-limited. While non-authenticated downloads from the API can be. |
c8d134c to
592d621
Compare
Codecov Report❌ Patch coverage is
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I quickly let copilot rebase the changes. (and fixed the cli bug) |
| default=4, | ||
| help="Number of allowed parallel tasks", | ||
| ) | ||
| @click.option( |
There was a problem hiding this comment.
I have some reservations regarding the--api-downloadoption from a user experience perspective. The current solution exposes implementation details that users shouldn't need to think about. Instead, we could focus on what the user actually wants to achieve - authenticated vs. anonymous downloads.
Consider something like --authenticated and a help text like Enable authenticated download (with better rate limits, access to private repos, etc.) instead.
Potentially, even --auth-method <method> to future-proof it, if we want to support multiple authentication methods in the future. For example, the option to download pipelines not hosted on GitHub has also been a long-standing request and similar features could then be successively added without renaming / replacing too many CLI arguments.
There was a problem hiding this comment.
@MatthiasZepper while I agree with the switch to authenticated, I am not sure if auth_method is helpful here, even in the future.
I think there is usually only one method for each SCM/git provider and even if there are more, you would require extensive logic to make sure that people do not use "github authentication" when actually wanting to download from gitlab.
- Rename --api-download to --authenticated for better UX - Replace os.rename with Pathlib operations - Refactor download_wf_files method to reduce code duplication - Rename compress to compress_type consistently across codebase - Update all references and tests accordingly
For repos that follow the nf-core template but need authentication.
PR checklist
CHANGELOG.mdis updateddocsis updated