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

Fix bug: add support for old and new Zenodo APIs #375

Merged
merged 8 commits into from
Oct 24, 2023
Merged

Conversation

santisoler
Copy link
Member

Allow the Zenodo downloader class to work both with the previous Zenodo API and the new API that is running since their migration to InvenioRDM on 2023-10-13. The downloader class ZenodoRepository determines which API version is interacting with and defines the download url and populate the registry based on that. Test if ZenodoRepository correctly supports both the old and new Zenodo APIs. Use pytest-httpserver to run a local server that returns API response that mimics the Zenodo ones. Define a base_api_url class attribute for ZenodoRepository so we can override it in tests.

Relevant issues/PRs:

Fixes #371

Related to #373

This PR was triggered by discussions on the Fatiando Meeting of 2023-10-19.

Update the Zenodo downloader to work with the new Zenodo API. Use the
`filename` key for getting the filenames of all files in a repository.
Update the generated download url based on heuristics: the link present
in the API reference doesn't work at the moment). Update the method that
populates the registry: use the new `filename` key and specify that the
checksums are now md5.
Detect which API are we interacting with and generate the download url
and/or populate the registry accordingly.
Test if `ZenodoRepository` correctly supports both the old and new
Zenodo APIs. Use `pytest-httpserver` to run a local server that returns
API response that mimics the Zenodo ones. Define a `base_api_url` class
attribute for `ZenodoRepository` so we can override it in tests.
@santisoler
Copy link
Member Author

@ericpre, @khaeru, @dokempf would you mind taking a look at this? I'm planning to merge this PR tomorrow.

@khaeru
Copy link

khaeru commented Oct 23, 2023

@santisoler thanks for the continued work on the issue by you and your colleagues. The strategy seems prudent to me: release code that tries to work regardless of whether Zenodo (a) stick with the API as it is currently operating or (b) manage to provide the backwards compatibility that they promised.

Per your comment here, it seems like we can't be 100% confident that they won't (c) make further changes that are neither (a) nor (b)—but it would be impossible to predict that, anyway, so this is the best possible action for the moment.

@santisoler
Copy link
Member Author

Thanks @khaeru. You're right, the best case scenario would be that Zenodo provide the details of the API they'll maintain in the long run. But considering all the issues they're trying to solve after the migration, we cannot be sure when it will happen.

So I think it's better to release some fixes that would Pooch work again to download files from Zenodo, until they sort this out. Since the only information we have so far on their intentions is to keep their API backward compatible, I think that allowing our code to interact with such API is a sensible decision.

I'll rerun all tests today to check that everything is working after the changes they introduced during the weekend and if all goes smoothly, tomorrow we'll have a new release of Pooch.

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.

KeyError: 'key' in ZenodoRepository.download_url() after Zenodo migration
2 participants