-
Notifications
You must be signed in to change notification settings - Fork 4
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
Use new Download API #54
Conversation
Nice, solid start. I'm inclined to create an I'll branch off here to try it out |
I updated this PR taking inspiration from yours. Please have a look at the CLI and the |
Hi, thanks for all the effort on this! I will take a look today or tomorrow 🤝 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks really great, thanks for all the hard work. I think we've ended up with something really clean.
I just saw one little typo that I've requested a change for, after that we can merge.
Once we've merged we may need a separate PR updating the docs, and I'd like to see if I can get my uv
PR in shape as well, and then we can go ahead and drop 1.0.0 I think.
Also something is up with the integration tests, looks like the service maybe being flaky but they are pretty red |
EEA updated the API schema and keep the version number.
on a different part of the docs says
Also, the Swager UI shows a new undocumented entry point However, the braking change is that it the dataset field went from accepting a list of integers to accepting a single integer. |
I wonder how unstable this thing is going to be... good thing the integration tests are actually catching this stuff though. Do you want to make those changes in this MR still? I can also push some commits to this branch if you want to hand off, you've done a lot already |
I'm motivated to see this trough, and need it for work... |
Co-authored-by: John Paton <john@johnpaton.net>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a few nits but I'm not worried about them, this has ended up really clean and well-structured. Great collab and great work, thanks so much as always. Let's get this merged! I'll let you do the honours
The new functionality for the new Parquet downloads API (see #52 for details) can be found on
airbase.download_api
:airbase.download_api.types
: type annotations for the API requestsairbase.download_api.dataset
: data structures representing an API dataset the data required for requesting URLsairbase.download_api.client
: single requests to the APIairbase.download_api.session
: machinery for requesting multiple URLs and downloading the filesOn the CLI we have 3 new sub commands:
airbase historical
: Historical Airbase data delivered between 2002 and 2012 before Air Quality Directive 2008/50/EC entered into force.airbase verified
: Verified data (E1a) from 2013 to 2022 reported by countries by 30 September each year for the previous year.airbase unverified
: Unverified data transmitted continuously (Up-To-Date/UTD/E2a) data from the beginning of 2023.Also
airbase.summary.DB
was updated to handle the information needed for the Parquet downloads APIstill missing:
airbase.AirbaseClient
andairbase.AirbaseRequest
, orairbase.AirbaseClient
andairbase.AirbaseRequest
to donwloading CSVs from the old API until EOL by the end of this year, or