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

Update CDS retreival to new API #52

Merged
merged 7 commits into from
Jan 7, 2025
Merged

Conversation

maxstb
Copy link
Contributor

@maxstb maxstb commented Jan 3, 2025

No description provided.

@maxstb
Copy link
Contributor Author

maxstb commented Jan 3, 2025

Since the API of the copernicus project was migrated, we need to update the julia package to the new API Version.
Information on how the new API is working can be found here: https://forum.ecmwf.int/t/ecmwf-apis-faq-api-data-documentation/6880

@maxstb maxstb force-pushed the master branch 2 times, most recently from 5904b1f to 59f0f51 Compare January 3, 2025 13:03
@juliohm
Copy link
Member

juliohm commented Jan 3, 2025

Thank you @maxstb, could you please confirm the error messages don't occur in your local setup with a API key?

@maxstb
Copy link
Contributor Author

maxstb commented Jan 3, 2025

I fixed tiny issues in the tests, but I cannot make them fully work locally.
The keyword for the format was changed to "data_format" and the "content_type" is no longer returned.

For the retrieve.jl test file, the second and third testsets are failing due to an unknown dataset and I cannot identify which were originally used. This might be due to the split into CDS and ADS datasets, but I am a bit lost there.

Regarding the pipelines authentication, I guess that a new account needs to be created with a new Authentication token. All old accounts do not work anymore, to my best knowledge. This could be another issue.

@juliohm
Copy link
Member

juliohm commented Jan 3, 2025

Is the old API still available? Or is it fully deprecated? If it is deprecated, we can proceed here and make the necessary changes to get up to date. If it is not deprecated, we need to add a version keyword argument to the retrieve function.

@maxstb
Copy link
Contributor Author

maxstb commented Jan 6, 2025

In this documentation (https://confluence.ecmwf.int/display/CKB/Please+read%3A+CDS+and+ADS+migrating+to+new+infrastructure%3A+Common+Data+Store+%28CDS%29+Engine) it's stated, that the old infrastructure is no longer accessible

@juliohm
Copy link
Member

juliohm commented Jan 6, 2025

Thank you for checking it. I will try to run the code locally by the end of the day with a new key.

@juliohm
Copy link
Member

juliohm commented Jan 6, 2025

@maxstb I managed to download data with a new CDS key 💯

Do you mind updating the tests here to consider new small datasets in the new API?

After you confirm the tests are ready, I can pull in the changes, test locally and merge.

@maxstb
Copy link
Contributor Author

maxstb commented Jan 7, 2025

Thank you!
I considered two new datasets which allow small data retrieval. Locally they work now for me 👍

@juliohm
Copy link
Member

juliohm commented Jan 7, 2025

Thank you @maxstb ! The last dataset is still failing for me. Could you please pull the latest changes and test locally again?

@maxstb
Copy link
Contributor Author

maxstb commented Jan 7, 2025

It's also working with your latest changes.
Can you check if the Pipeline CDS Account has accepted the terms of use for this dataset (https://cds.climate.copernicus.eu/datasets/ecv-for-climate-change?tab=download)

@juliohm
Copy link
Member

juliohm commented Jan 7, 2025

The terms are accepted here too:

image

@maxstb
Copy link
Contributor Author

maxstb commented Jan 7, 2025

Can you provide me the error output?

@juliohm
Copy link
Member

juliohm commented Jan 7, 2025

The issue is in the run(tar ...) command. Could you please rewrite it using more portable tools like https://github.com/JuliaIO/Tar.jl?

@juliohm
Copy link
Member

juliohm commented Jan 7, 2025

Can you provide me the error output?

tar: This does not look like a tar archive
tar: Skipping to next header
tar: Exiting with failure status due to previous errors
ERROR: failed process: Process(tar -xzvf test/data/ecc.tar.gz -C test/data/ecc, ProcessExited(2)) [2]

@maxstb
Copy link
Contributor Author

maxstb commented Jan 7, 2025

I had some problems using the Tar package, that's why I tried using the "run" command, but I will try doing it again later on. 👍

@juliohm
Copy link
Member

juliohm commented Jan 7, 2025

We could also remove this dataset from the test suite. The API is being tested with 3 datasets already. Should we simply do that?

@maxstb
Copy link
Contributor Author

maxstb commented Jan 7, 2025

Sounds fine to me. I removed the test.
I guess that the tgz format is not really supported by CDS anymore. At least, I cannot find any up-to-date information on this

@juliohm
Copy link
Member

juliohm commented Jan 7, 2025

Thank you @maxstb. If you have some time available, it would be awesome to fix #46

@juliohm juliohm merged commit f29c007 into JuliaClimate:master Jan 7, 2025
0 of 6 checks passed
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.

2 participants