Skip to content

Feature/download resume dataset #542

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
merged 2 commits into from
Apr 29, 2025
Merged

Conversation

aaperis
Copy link
Contributor

@aaperis aaperis commented Apr 24, 2025

Related issue(s) and PR(s)
This PR closes #405.

Description

If the flag -continue is given to download then any existing file with the same name and file path will be skipped during download.

How to test
See that the added integration tests pass.

@aaperis aaperis requested a review from a team April 24, 2025 20:09
@aaperis aaperis self-assigned this Apr 24, 2025
Copy link
Member

@MalinAhlberg MalinAhlberg left a comment

Choose a reason for hiding this comment

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

All looks well to me, and works fine when tested.

I get a bit confused by the name continue, however. It sounds like that without this flag, the program would stop when trying to download files that already exists. Maybe we could make this clearer in the docs? (sorry for not realizing and raising this earlier)

@aaperis
Copy link
Contributor Author

aaperis commented Apr 29, 2025

All looks well to me, and works fine when tested.

I get a bit confused by the name continue, however. It sounds like that without this flag, the program would stop when trying to download files that already exists. Maybe we could make this clearer in the docs? (sorry for not realizing and raising this earlier)

I can see why it may seem confusing and I agree that this is not ideal. I wanted to avoid using -resume because this could be perceived as "resuming a partially downloaded file" whereas here this is not the case. We also use -continue for a similar functionality in the Upload module so I thought this would be at least consistent.
Given that this is the first iteration for implementing this functionality (I would prefer to check more than the filenames), I say we go with it for now, and I open an issue for a follow-up that includes the naming of the flag (#544).

@aaperis aaperis added this pull request to the merge queue Apr 29, 2025
Merged via the queue into main with commit 654ae8a Apr 29, 2025
6 checks passed
@aaperis aaperis deleted the feature/download-resume-dataset branch April 29, 2025 11:44
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.

[download] Resume download of a dataset
3 participants