Skip to content
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

Changed the algorithm to fetch the download filename #1146

Merged
merged 7 commits into from Feb 3, 2017
Merged

Conversation

peristeri
Copy link
Contributor

Replace the regex with the parse_header function. Also removed the old
download function.

Fixes #1144

Replace the regex with the parse_header function. Also removed the old
download function.
@peristeri peristeri added this to the Release 3.0 milestone Jan 31, 2017
@peristeri peristeri added the bug category: fixes an error in the code label Jan 31, 2017
Copy link
Contributor

@SaraDupont SaraDupont left a comment

Choose a reason for hiding this comment

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

It would be nice to have an output when downloading a dataset just to confirm the download has started. For example: Downloading sct_example_data ...


printv('Copy binaries to %s\n' % dest_folder, verbose)
Copy link
Contributor

Choose a reason for hiding this comment

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

Some of the data downloadable by sct_download_data are not binaries (for ex PAM50 and MNI-Poly-AMU are templates, sct_example_data and sct_testing_data are example/testing datasets, etc)

Maybe just print Copy files to xx or Copy data to xx

@jcohenadad
Copy link
Member

jcohenadad commented Feb 1, 2017

@peristeri: in case of internet glitch the previous function could handle that-- is it still the case with your modification?

Copy link
Member

@jcohenadad jcohenadad left a comment

Choose a reason for hiding this comment

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

please see my comment about internet glitch

@peristeri
Copy link
Contributor Author

@jcohenadad if you are refering to retries on failed attempts?

Added additional redundancy by retrying download if there's a server or
connection error.
@jcohenadad
Copy link
Member

yes, retry if connection pb

@peristeri peristeri merged commit 9db81be into master Feb 3, 2017
@peristeri peristeri deleted the fix-download branch February 3, 2017 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug category: fixes an error in the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants