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

More informative error message when resolving DOI URLs #456

Open
remrama opened this issue Dec 29, 2024 · 2 comments
Open

More informative error message when resolving DOI URLs #456

remrama opened this issue Dec 29, 2024 · 2 comments

Comments

@remrama
Copy link

remrama commented Dec 29, 2024

Hi, we have an open PR for the YASA library at raphaelvallat/yasa#192 that transitions from test files being shipped with our repository to using Pooch to download them. Everything is working well and all tests are now passing (Python versions 3.9-3.12 on ubuntu, macos, and windows). However, the initial tests failed on 2 of the 12 systems (3.11 and 3.12 macos). The failing tests passed when we reran them after 24 hours without changing any code, so I presume it was a temporary connection- or server-related issue.

The error raised during tests was from L648-655 in downloaders.py, where Pooch tries to resolve the DOI, despite the DOI existing and passing in other tests.

ValueError: 'Archive with doi:10.5281/zenodo.14564284 not found (see https://zenodo.org/doi/10.5281/zenodo.14564284). Is the DOI correct?'

I have 2 questions:

  1. Have you had similar experiences when running CI tests with Pooch downloads? If so, do you have any general advice on how best to handle potential connection issues when downloading files during CI tests (e.g., caching)?
  2. Would it be possible to make that DOIDownloader error message more informative? Currently it clusters all status codes 400-600 into the assumption that the DOI was wrong. I think it would be helpful if the response code itself was provided in the error message to help the user determine why the DOI was not resolved (e.g., client vs server issues). I would be willing to submit a small PR if this makes sense to add.
@ophrys97
Copy link

I encountered the same issue. I had to bypass the DOI and download the files manually. However, I'm not sure if this is a temporary issue related to the server or connection. I will try again tomorrow to confirm. I hope the developers will address and resolve this issue as soon as possible.

@santisoler
Copy link
Member

Hi @remrama. Thanks for opening this issue.

It's not rare to encounter failed downloads when running tests on CI. Most of the times this is related to server-side issues, usually because we trigger multiple downloads in parallel. My pragmatic approach is to split jobs in Actions so I can rerun the failed ones without having to rerun all the tests. Another approach would be to include some sleeps in between jobs, so they are not triggered at the same time, although tests will take a little bit longer to run (I haven't experimented with that).

  1. Would it be possible to make that DOIDownloader error message more informative? Currently it clusters all status codes 400-600 into the assumption that the DOI was wrong. I think it would be helpful if the response code itself was provided in the error message to help the user determine why the DOI was not resolved (e.g., client vs server issues). I would be willing to submit a small PR if this makes sense to add.

I totally agree that we should improve the error messages. Also, grouping together all codes from 400-600 is not the best approach. Printing out more information about the status code would be also great to troubleshoot errors. Should we create categories for different error codes? For example, I think status codes like 403, 404, 429, 502, 503, 504 could be filtered out and have personalized error messages for each one of them, while we can have a template for the other ones in range 400-600. Would love to hear ideas to improve this!

BTW, if anyone wants to work on this, please let us know! We can decide on some ideas and then anyone can open a PR to implement them.

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

No branches or pull requests

3 participants