Skip to content

Support async refreshable credential callbacks in Python bindings#1639

Open
mpiannucci wants to merge 17 commits intomainfrom
async-creds-python
Open

Support async refreshable credential callbacks in Python bindings#1639
mpiannucci wants to merge 17 commits intomainfrom
async-creds-python

Conversation

@mpiannucci
Copy link
Copy Markdown
Collaborator

@mpiannucci mpiannucci commented Feb 12, 2026

Adds support for async refreshable credential callbacks in the Python bindings, allowing users to provide async def functions for credential refresh that work correctly across sync and async contexts via pyo3-async-runtimes' TaskLocals bridge.

Closes #1424

@mpiannucci mpiannucci marked this pull request as ready for review February 12, 2026 22:23
@mpiannucci mpiannucci requested a review from li-em February 13, 2026 14:42
Copy link
Copy Markdown
Collaborator

@paraseba paraseba left a comment

Choose a reason for hiding this comment

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

This is a miracle of engineering hehe, I understand little of it, but that's OK. I hope we can make Samantha's tests pass and merge.

return asyncio.run(coroutine)
except RuntimeError as err:
coroutine.close()
if "asyncio.run() cannot be called from a running event loop" in str(err):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Interesting.... this is a bit of a breaking change, because everybody (including Earthmover) uses True for scatter

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'll think on this..

mpiannucci and others added 9 commits February 16, 2026 10:01
- When no Python event loop is running, create a persistent fallback
  loop on a background thread instead of calling asyncio.run() each
  time (which creates a new temporary loop per invocation).
- Detect potential deadlocks when the credential callback targets the
  same event loop that is currently blocked, and raise an error
  instead of hanging.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@mpiannucci
Copy link
Copy Markdown
Collaborator Author

mpiannucci commented Feb 16, 2026

This is working well now, but it required adding throwing an exception when calling synbc api from async context

I think we need to change this to warn that this is can lead to deadlocks and that in a future version of icechunk this behavior may be disallowed.

A good example of the consequence of this is the xarray test failure

@mpiannucci
Copy link
Copy Markdown
Collaborator Author

@dcherian i dont understand the xarray test suite, do you ahve any context on this?

________________ TestIcechunkStoreFilesystem.test_load_async[3] ________________

self = <tests.run_xarray_backends_tests.TestIcechunkStoreFilesystem object at 0x7ff67a78a050>

    @pytest.mark.asyncio
    @pytest.mark.skipif(
        not has_zarr_v3,
        reason="zarr-python <3 did not support async loading",
    )
    async def test_load_async(self) -> None:
>       await super().test_load_async()

.venv/lib/python3.11/site-packages/xarray/tests/test_backends.py:2659: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
.venv/lib/python3.11/site-packages/xarray/tests/test_backends.py:595: in test_load_async
    with assert_loads() as ds:
../../../../_temp/uv-python-dir/cpython-3.11.14-linux-x86_64-gnu/lib/python3.11/contextlib.py:137: in __enter__
    return next(self.gen)
           ^^^^^^^^^^^^^^
.venv/lib/python3.11/site-packages/xarray/tests/test_backends.py:583: in assert_loads
    with self.roundtrip(expected) as actual:
../../../../_temp/uv-python-dir/cpython-3.11.14-linux-x86_64-gnu/lib/python3.11/contextlib.py:137: in __enter__
    return next(self.gen)
           ^^^^^^^^^^^^^^
.venv/lib/python3.11/site-packages/xarray/tests/test_backends.py:2648: in roundtrip
    with self.create_zarr_target() as store_target:
../../../../_temp/uv-python-dir/cpython-3.11.14-linux-x86_64-gnu/lib/python3.11/contextlib.py:137: in __enter__
    return next(self.gen)
           ^^^^^^^^^^^^^^
tests/run_xarray_backends_tests.py:51: in create_zarr_target
    with self.create_repo() as repo:
../../../../_temp/uv-python-dir/cpython-3.11.14-linux-x86_64-gnu/lib/python3.11/contextlib.py:137: in __enter__
    return next(self.gen)
           ^^^^^^^^^^^^^^
tests/run_xarray_backends_tests.py:102: in create_repo
    yield Repository.create(local_filesystem_storage(tmpdir))
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

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.

Allow async credential refresh functions

3 participants