zenoh
zenoh copied to clipboard
feat: allow subscriber to be run in background
Background subscribers have their lifetime bound to the session one. They are undeclared when the session is closed, not when they are dropped.
This is a quick feature proposal I've in mind this morning. It's mainly meant to solve "the subscriber api not being pythonic" issue, but it may also be convenient for Rust users too. The principle would of course be extended to Queryable.
The idea for the Python bindings is to use background=true by default, and set background to false when using the context manager form.
@kydos @Mallets @milyin @DenisBiryukov91
Actually, I've realized I don't need to set background to false when using context manager, because even if background is true, Subscriber::undeclare can still be called.
@wyfo Please change the base branch to dev/1.0.0 as protocol_changes is no longer the development branch.
As this feature is important for Python semantic I think it should be added. This is an update to Rust API so it have to be additionally decided, do we need it in Rust and other bindings? So at this moment I propose to put it under "unstable" feature and later we can decide, do we need to make it part of API or just put it under "internal" mod to use it for Python only. @Mallets, what do you think?
I've recently realized that I would still have to maintain an internal pool of objects (subscribers/queryables/etc.) in the Python session wrapper because of the order of destruction of Python objects (session would still be destroyed first because declared first, so it would need to drop dependent subscribers/queryables/etc. first, because they still maintain a reference on the session). So I should be able to implement the feature in Zenoh Python without having it in Rust. But as it is a semantic addition to the API, I would prefer to have it in the Rust API too, as I don't think semantic additions in bindings are a good thing. Moreover, it would still be better for the implementation on Python side, as it would avoid to juggle with blocking destructors of dependent objects, only using the session one.
I think this doesn't change a lot. This is not a semantic change, this is just addition which is necessary for python and may be useful for other languages, but this is not decided yet. So I don't see any problem to implement it and put to unstable for now
I think you misunderstood. I need an internal pool to allow
with zenoh.open(...) as session:
sub = session.declare_subscriber(...)
to work, but it does not change the fact that session.declare_subscriber(...) without variable declaration (or context manager) should be allowed to work, because the opposite is completely unpythonic.
As I wrote in the issue:
It's mainly meant to solve "the subscriber api not being pythonic" issue, but it may also be convenient for Rust users too.
The pseudo-RAII issue of Python (requiring the internal pool) is a Python specific problem and is orthogonal to the unpythonicity one.
Am I correct that this feature is required to write:
with zenoh.open(...) as session:
session.declare_subscriber(...)
and be sure that subscriber works inside the whole with block, not dropped immediately after declare_subscriber line ?
This feature is not "required", as I can find a way to make it work in my implementation. However, it is required to have a semantic consistency between the APIs.
This feature is not "required", as I can find a way to make it work in my implementation. However, it is required to have a semantic consistency between the APIs.
Ok, understand. But statements below are still true, correct?
- this feature (keeping object alive until session is dropped) is required for python to keep code idiomatic. Specifically, the RAII concept is not so popular in python, so users can easily forget to assign subscriber to variable to keep it alive. And there is no way to ensure that user have to assign return value from "declare_subscriber"
- it's easier to implement this functionality in Python if it's already provided by Rust. Though it also can be implemented separately
So I think the right way is to provide background entities from Rust as "unstable" and later decide, should they go to the main api or should it be moved to "internal" as binding-specific feature
Specifically, the RAII concept is not so popular in python, so users can easily forget to assign subscriber to variable to keep it alive.
There is no RAII in Python, it's not about popularity. Having a finalizer that may be executed at the end of a function, if there is no exception, and in random order is not RAII, and it's not something reliable. That's why you cannot replace a context manager with a finalizer, and that's why you would expect such behavior of having to declare a variable.
And there is no way to ensure that user have to assign return value from "declare_subscriber"
You don't want to know possible ways of doing it. (It's Python, almost everything is possible 🙂)
it's easier to implement this functionality in Python if it's already provided by Rust. Though it also can be implemented separately
Correct.
I think we can go ahead with this feature under an unstable feature gate. Similar approach should be applied to declare_queryable.
I think we can go ahead with this feature under an
unstablefeature gate. Similar approach should be applied todeclare_queryable.
Agree. @wyfo please implement it on Rust for all necessary entities under [unstable]
Superseded by #1364