Skip to content
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

Python AsyncClient and JS ProxySession #2929

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tomjakubowski
Copy link
Contributor

@tomjakubowski tomjakubowski commented Feb 21, 2025

Pull Request Checklist

  • Add support in perspective-js for creating proxy sessions from a
    connected client.
  • Support for running maturin commands with uv
    • set PSP_UV=1 in environment
    • use uv venv to create venv, then activate it
    • uv pip install -r **/requirements.txt to install requirements
  • Update pyo3 from 0.21 to 0.23
    • this was to support use of the pyo3-async-runtimes crate
  • Build abi3 .so when building with maturin develop
    • Caveat: after this change, old non-abi3 .so's still lingering in
      the rust/perspective-python directory may be loaded preferentially
      by Python when using an editable install. It is best to delete
      those old .so's.
  • Repurpose PyClient as AsyncClient and export bindings to Python
  • Add async client pytests for both cpython and pyodide
    • Uses pytest-asyncio plugin
  • Add python and pytest vscode settings
    • Supports running cpython pytest suite from vscode's Test Explorer
  • Description which clearly states what problems the PR solves.
  • Description contains a link to the Github Issue, and any relevent
    Discussions, this PR applies to.
  • Include new tests that fail without this PR but passes with it.
  • Include any relevent Documentation changes related to this change.
  • Verify all commits have been signed in accordance with the DCO policy.
  • Reviewed PR commit history to remove unnecessary changes.
  • Make sure your PR passes build, test and lint steps completely.

Add support in perspective-js for creating proxy sessions from a
connected client.

Signed-off-by: Tom Jakubowski <[email protected]>
@tomjakubowski tomjakubowski force-pushed the feature/py-async-client branch from 65c8f99 to 4b2fbce Compare February 21, 2025 03:28
* Support for running maturin commands with uv
  * set `PSP_UV=1` in environment
  * use `uv venv` to create venv, then activate it
  * `uv pip install -r **/requirements.txt` to install requirements
* Update pyo3 from 0.21 to 0.23
  * this was to support use of the `pyo3-async-runtimes` crate
* Build `abi3` .so when building with `maturin develop`
  * **Caveat**: after this change, old non-abi3 .so's still lingering in
    the rust/perspective-python directory may be loaded preferentially
    by Python when using an editable install.  It is best to delete
    those old .so's.
* Repurpose PyClient as AsyncClient and export bindings to Python
* Add async client pytests for both cpython and pyodide
  * Uses `pytest-asyncio` plugin
* Add python and pytest vscode settings
  * Supports running cpython pytest suite from vscode's Test Explorer

Signed-off-by: Tom Jakubowski <[email protected]>
@tomjakubowski tomjakubowski force-pushed the feature/py-async-client branch from 4b2fbce to 7974866 Compare February 21, 2025 03:35
@tomjakubowski
Copy link
Contributor Author

I was seeing some new error spew in the pyodide tests and I think there's a bug in pytest-pyodide, filed here pyodide/pytest-pyodide#148

@tomjakubowski tomjakubowski marked this pull request as ready for review February 21, 2025 04:27
})?;
if let Some(fut) = Python::with_gil(move |py| -> PyResult<_> {
let ret = handle_request.call1(py, (PyBytes::new(py, msg),))?;
if isawaitable(ret.bind(py)).unwrap_or(false) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Crucial change here: if calling the Python callback returns a coroutine (i.e. if the callback is a coroutine function), then convert the coro into a Rust future and then await it (line 72)

@tomjakubowski tomjakubowski mentioned this pull request Feb 21, 2025
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.

1 participant