Skip to content

Use xarray load_async to make WMS router async#136

Open
TomNicholas wants to merge 7 commits into
xpublish-community:mainfrom
TomNicholas:async_xarray_load
Open

Use xarray load_async to make WMS router async#136
TomNicholas wants to merge 7 commits into
xpublish-community:mainfrom
TomNicholas:async_xarray_load

Conversation

@TomNicholas
Copy link
Copy Markdown

@TomNicholas TomNicholas commented May 19, 2025

Builds on @mpiannucci 's PR #98 but instead uses the (as yet unmerged) xarray PR pydata/xarray#10327 which adds da.load_async, which should work when lazily loading zarr data using zarr-python v3 without dask.

This also introduces clipping for GetMap requests

I deleted this part of #98 because it seemed separable.

@TomNicholas TomNicholas changed the title Use xarray load_async Use xarray load_async to make WMS router async May 19, 2025
Comment on lines 45 to +47
case WMSGetCapabilitiesQuery():
return get_capabilities(dataset, request, query)
return await asyncio.to_thread(get_capabilities, dataset, request, query)
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really understand the context in which these other functions get called - is it important to make these properly async too?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My guess is that it's only functions which actually trigger loading of additional chunk data which need to be fully async - if all they do is process metadata that should already be in memory then they don't need to be async.

Copy link
Copy Markdown
Collaborator

@mpiannucci mpiannucci May 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once getmap is good, we need to make all the other methods also load data async instead of using to thread.

They all load data just a little differently than getmap

@TomNicholas
Copy link
Copy Markdown
Author

Presumably the CI tests did not run on this PR because I'm a new contributor?

@srstsavage srstsavage closed this May 19, 2026
@srstsavage srstsavage reopened this May 19, 2026
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.

3 participants