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

[Bug/Breaking Change] session.get tuple callback no longer works in 1.0.0 #364

Closed
fsteff opened this issue Oct 18, 2024 · 1 comment · Fixed by #366
Closed

[Bug/Breaking Change] session.get tuple callback no longer works in 1.0.0 #364

fsteff opened this issue Oct 18, 2024 · 1 comment · Fixed by #366
Labels
bug Something isn't working

Comments

@fsteff
Copy link

fsteff commented Oct 18, 2024

Describe the bug

Is it on purpose that the option of passing a tuple as session.get handler no longer works in 1.0.0?
Instead it is necessary to wrap it into a handlers.Callback (i expected that this would happen automatically)

IMHO that's an unnecessary breaking change and I could not find any documentation about this change.
At least this should be documented in the 0.11.0 -> 1.0.0 migration section. Thanks :-)

To reproduce

import zenoh

session = zenoh.open(zenoh.Config())

def on_reply(reply):
    print('reply')

def on_done():
    print('done')

def on_query(query: zenoh.Query):
    query.reply(query.key_expr, 'test')

qable = session.declare_queryable('test', on_query)
session.get('test', (on_reply, on_done))

result: only "reply" is written to the console

workaround / breaking change :

 session.get('test', zenoh.handlers.Callback(on_reply, on_done))

System info

  • zenoh version: 1.0.0-rc.2
@fsteff fsteff added the bug Something isn't working label Oct 18, 2024
@fsteff fsteff changed the title [Bug/Breaking Change] session.get tuple callback no longer works [Bug/Breaking Change] session.get tuple callback no longer works in 1.0.0 Oct 18, 2024
@wyfo
Copy link
Contributor

wyfo commented Oct 18, 2024

The Python bindings have been rewritten from scratch for 1.0, trying to match more carefully the Rust API. In Rust, a tuple handler has the meaning (callback, handler), not (callback, done_callback).
I think it's too late to rollback to the old behavior, but that's indeed a breaking change that should be mentioned in bold in the migration guide.

P.S. Looking again at the 0.11 version, the signature was quite confusing IMO, as you were able to pass a tuple (closure, receiver), but also a tuple (callback, done_callback). So I would say the change was for the better, but I acknowledge that it's annoying.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants