python: Add a server listener that handles client message callbacks #226
Conversation
| #[pyo3(get)] | ||
| id: u32, | ||
| #[pyo3(get)] | ||
| topic: String, |
There was a problem hiding this comment.
This will create a Python string on every access, and involve at least two copies. One from rust str to owned string and a second on access to python string.
I don't think it will matter much to performance, since these are tiny, but we could save one copy by storing this a Python string and doing the conversion upfront.
Thoughts? We don't have to do this now, it's not a breaking change.
I'm more concerned about setting a good example for future code that we write than this particular case.
There was a problem hiding this comment.
👍 I changed this to PyString and now construct that directly from the &str.
| }; | ||
|
|
||
| let result: PyResult<()> = Python::with_gil(|py| { | ||
| let kwargs = PyDict::new(py); |
There was a problem hiding this comment.
We're creating this temporary dict here to call every callback. Why not use positional arguments?
The user can still choose to use kwargs if they want, but then at least they're aware of the overhead they're adding.
| host: &str, | ||
| port: u16, | ||
| capabilities: Option<Vec<PyCapability>>, | ||
| server_listener: Option<Py<PyAny>>, |
There was a problem hiding this comment.
If we're taking a PyAny here, why do we need to export PyServerListener to Python? I don't think we need it all?
There was a problem hiding this comment.
Good catch, thanks. Leftover from before I realized it had to be implemented in Python directly.
bryfox
deleted the
bryan/fg-10180-python-register-a-server-listener-with-client_message
branch
This adds a server listener interface to the Python SDK.
This is implemented as a Protocol in python; pyo3 does not support extending a Python class. To provide reasonable typing on
start_server, that's now a wrapping function in the public package, to avoid the stub interface referencing the external type.Since the ws-related code is starting to grow in python, I've moved related classes and functions into
websocket_server.rs(from lib.rs). The exports are still in the top-levelfoxglovepackage for now.To support the message data callback, this also adds support for the
ClientPublishcapability andsupported_encodings.I updated the existing live_visualization example to exercise this.