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

Callbacks fail when they are methods #667

Closed
quantotto opened this issue Mar 10, 2021 · 3 comments
Closed

Callbacks fail when they are methods #667

quantotto opened this issue Mar 10, 2021 · 3 comments

Comments

@quantotto
Copy link

In the 0.58.0 release, websocket-client changed the way it handles callbacks, such as on_open, on_message.

Callbacks that are methods of a class are not supported now. Previously, there was a check in _app.py:

 def _callback(self, callback, *args):
        if callback:
            try:
                if inspect.ismethod(callback):
                    callback(*args)
                else:
                    callback(self, *args)

Now, starting from 0.58.0, it simply calls callback(*args) unconditionally.

Is there any reason for this change and does it mean that websocket-client drops support for method callbacks intentionally.

Thanks!

@quantotto quantotto changed the title Callbacks fails when they are methods Callbacks fail when they are methods Mar 10, 2021
@engn33r
Copy link
Collaborator

engn33r commented Mar 10, 2021

Glad you asked - please see the history for this issue outlined in commit 3112b7d. If you search existing issues, many are related to this callback functionality, and PR #442 caused quite a few of those problems. I would be happy to consider a PR that provides such functionality without creating the issues that PR #442 did.

@quantotto
Copy link
Author

Thank you for the details. I guess I didn’t search the history enough, just looked at issues filed in 2021.

Such a change breaks some interface compatibility and I thought this would be in a major (1.x release), but I understand that it reverts some other major changes.

Meanwhile, I have to revert to 0.57.0. As for PR, I have nothing to suggest at the moment as it would take time to digest / outline design requirements and various use cases.

@engn33r
Copy link
Collaborator

engn33r commented Mar 10, 2021

I don't disagree with what you've said. This callback function has been a messy topic in the past, and arguably the introduction of it should have been the 1.x release and the reversion the 2.x release. Proper semantic versioning has been lacking in this project and I'll aim to improve that going forward. I'll consider this particular issue resolved for the moment since we have identified the root cause, but we can continue the discussion if I left something out.

As mentioned, the inheritance introduced by PR #442 could be a useful feature to add if someone wants to attempt an implementation that can avoid the pitfalls observed previously.

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

No branches or pull requests

2 participants