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

adding ability to call instance method as callback #510

Closed
wants to merge 1 commit into from

Conversation

ofirule
Copy link

@ofirule ofirule commented Dec 9, 2018

If I understand correctly, when someone inherits from WebSocketApp, and uses an instance method as a callback there is no reason to pass 'self' again to the callback.
but if someone uses an instance method of some other class the callback will fail with error:
on_open() missing 1 required positional argument: 'ws'
or similar

My fix allows passing 'self' only once when callback is an instance method of WebSocketApp, but won't fail when the callback is an instance of another class

I saw it was already part of the code once :
ae58949

If I understand correctly, when someone inherits from WebSocketApp, and uses an instance method as a callback there is no reason to pass 'self' again to the callback.
but if someone uses an instance method of some other class the callback will fail with error:
on_open() missing 1 required positional argument: 'ws'
or similar

My fix allows passing 'self' only once when callback is an instance method of WebSocketApp, but won't fail when the callback is an instance of another class

I saw it was already part of the code once :
websocket-client@ae58949
@jhtitor
Copy link
Contributor

jhtitor commented Dec 19, 2018

This should be merged ASAP, latest websocket-client release is breaking compatibility with older code without this fix.

Copy link

@rad-pat rad-pat left a comment

Choose a reason for hiding this comment

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

This looks like what is required for me to be able to migrate from 0.48.0 to current. At the moment, backwards compatibility is broken and I may only upgrade as far as 0.52.0.

@engn33r
Copy link
Collaborator

engn33r commented Feb 15, 2021

This is a duplicate of PR #462. However, this callback branch should have been removed in the process of reverting PR #442. However, the revert was not done cleanly, so commit 3112b7d finally removes this callback branch and brings the callback back to where it was before PR #442. Due to commit 3112b7d, this PR is not longer relevant. If you would like to see or comment on the full history on this topic, please see this comment on commit 3112b7d.

@engn33r engn33r closed this Feb 15, 2021
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.

None yet

4 participants