chitralverma commented on PR #6622: URL: https://github.com/apache/opendal/pull/6622#issuecomment-3402276927
Hello team, I'm providing an update on the refactoring effort. While I've completed about 70% of the work, I've hit a significant roadblock with the `async` methods in `AsyncOperator`, `AsyncLister`, and `AsyncFile`. The issue stems from a fundamental mismatch between how `opendal-python` handles asynchronous operations and how `pyo3-stub-gen` expects them to be structured. ### The Core Issue: A Mismatch in Async Handling 1. **`pyo3-stub-gen`'s Expectation**: To generate correct `async def` stubs, the tool requires that the Rust functions themselves are declared as `async fn`. This typically involves enabling PyO3's `experimental-async` feature. 2. **OpenDAL's Implementation**: We use the recommended pattern from `pyo3-async-runtimes`, where a **synchronous** Rust function acts as a "factory" that returns a Python future (by calling `future_into_py`). This is [the correct way](https://github.com/PyO3/pyo3-async-runtimes/issues/67#issuecomment-3401453581) to avoid `!Send` lifetime issues when bridging Rust async code to Python's event loop. Because our Rust functions are not literally `async fn`, `pyo3-stub-gen` generates incorrect synchronous stubs (e.g., `def read(...)` instead of `async def read(...)`). This defeats the purpose of type hinting for our entire async API. ### Possible Paths Forward I see two potential solutions to this problem: #### 1. Overhaul OpenDAL's Async Implementation (Not Recommended) We could refactor all our async methods to use `async fn` in Rust, likely by creating a custom wrapper around the Tokio runtime. I've drafted a proof-of-concept for this below. However, I **won't recommend** this as it would be a substantial and invasive change to our core async logic, introducing complexity and potential risks, purely for the sake of stubs. <details> <summary><strong>Click to see</strong> a proof-of-concept `future_into_py_async` wrapper</summary> ```rust /// Spawns a future on the shared Tokio runtime and returns a Python awaitable. /// Relies on the runtime from `pyo3-async-runtimes`. pub async fn future_into_py_async<F, T>(fut: F) -> PyResult<T> where F: std::future::Future<Output = PyResult<T>> + Send + 'static, T: Send + 'static, { let rt = pyo3_async_runtimes::tokio::get_runtime(); let handle = rt.handle().clone(); // Use a channel to bridge the result back from the spawned task let (tx, rx) = tokio::sync::oneshot::channel(); // Spawn the future on the Tokio runtime handle.spawn(async move { let result = fut.await; let _ = tx.send(result); }); // Await the result from the channel match rx.await { Ok(result) => result, Err(e) => Err(crate::errors::Unexpected::new_err(format!( "Async task panicked or was dropped: {e}" ))), } } ``` </details> #### 2. Enhance `pyo3-stub-gen` (Recommended Path) ✅ The more pragmatic and correct solution is to address this limitation in the stub generation tool itself. The tool should provide a way to manually hint that a synchronous Rust function produces an awaitable. To that end, **I have already opened an issue on the `pyo3-stub-gen` repository** requesting this feature: - **[Issue #326: Add attribute to mark sync functions returning futures as `async def`](https://github.com/Jij-Inc/pyo3-stub-gen/issues/326)** **Side Note:** The recent migration from `chrono` to `jiff` will require a separate, smaller issue to be filed with `pyo3-stub-gen` to add support for `jiff::Timestamp`. I will handle that. Looking forward to your thoughts on this approach. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
