Allow nf-core download -r to download commits
#2906
Conversation
|
@nf-core-bot fix linting |
fellen31
force-pushed
the
download_commits
branch
from
74bacb8 to
937e442
Compare
|
@nf-core-bot fix linting |
| # Get commit information from github api | ||
| commit_response = gh_api.safe_get(f"https://api.github.com/repos/{pipeline}/commits?sha={branch['name']}") | ||
| for commit in commit_response.json(): | ||
| wf_commits.append(commit["sha"]) |
There was a problem hiding this comment.
I am a bit scared of this - is it really getting every single commit from a repo? In some repos that could be a huge response.
I wonder if we move this into a separate function call and only do it if we need to - that is, once we're sure that the user is passing a commit and is not going to use a branch or a tag?
I suspect that the number of people wanting exact commits to be a tiny proportion, so I don't want to slow down / risk crashes for the majority.
ewels
added
the
awaiting-changes
|
On hold until further notice. Nearly complete in case anyone wants to pick it up. Suggestion is to guess the URL for the supplied commit (if it looks like a hash - is the right length and characters) and try it, see if we get a 404 or not. That way we don't need to fetch the commit history for the repo. |
|
This PR has been tagged as awaiting-changes or awaiting-feedback by an nf-core contributor. Remove stale label or add a comment if it is still useful. |
Mimic the behaviour of
nextflow run -rand allow selected commit(s) to be downloaded.https://nfcore.slack.com/archives/CE5LG7WMB/p1712143905189639
Not sure about the effect it will have on Tower downloads.
PR checklist
CHANGELOG.mdis updateddocsis updated