Add batch download tables - #331
Conversation
Add private helpers for preparing download contexts and loading a prepared table, then route single-table downloads through them.
Prepare all table download contexts with one metadata request, then load unique tables concurrently.
c3d68d0 to
f5912b0
Compare
tim-hamblin-seequent
left a comment
There was a problem hiding this comment.
A few comments. I think this make sense to me after having a dig into it: you are leveraging the fact that prepare_data_download offers the ability to specify multiple data_identifiers and you are exposing it via download_tables. This seems all good to me, but it will need the check off from a maintainer.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Let TaskGroup coordinate table download tasks and propagate failures while preserving bounded concurrency.\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
peter-kipping-seequent
left a comment
There was a problem hiding this comment.
Adding some comments to the PR, however the main issue is actually that this approach isnt viable in 3.10, which we formally support on this SDK.
Taskground was introduced in 3.11, do you'll have to change the implementation of this to a 3.10 compatible concept.
| downloads = await self._prepare_data_downloads(object_id, version_id, [data_id]) | ||
| return await self._download_prepared_table(downloads[data_id], table_info, fb) | ||
|
|
||
| async def _prepare_data_downloads( |
There was a problem hiding this comment.
Im not the biggest fan on this function name, especially since its just shadowing the actual 'prepare_data_downloads' on a client.
However i cant really think of a better one right now.
If someone has a better suggestion im all ears, but what does 'prepare data downloads' mean?
Isnt it more like 'fetch list of downloads" or something?
|
Hey Peter, I've implemented your suggestions and used an async method compatible with 3.10. Cheers. |
Description
This PR aims to address the slow download issue raised here.
For a Geoscience Object that contains many tables(i.e 100 tables) fetching that data using
download_tableis very slow as it will make a metadata request and fetch the object definition each time for every table to prepare the context for downloading. To address this, I've introduced a new methoddownload_tablesthis will make a single metadata request to prepare the context for downloading, and the specified tables will be downloaded concurrently.Checklist