Skip to content

add on_open() and on_close() methods, similar to tornado web socket implementation? #52

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

Closed
songololo opened this issue Apr 8, 2015 · 5 comments

Comments

@songololo
Copy link

Would it be possible to add high-level on_open() and on_close() methods, similar to the web sockets implementation in Tornado, so that initialisation and cleanup can be performed?

@aaugustin
Copy link
Member

Perhaps there's a more straightforward solution for websockets, as its API uses coroutines rather than callbacks:

def handler(ws, path):
    do_on_open(ws)
    try:
        # handler logic.
        do_handler(ws, path)
    finally:
        do_on_close(ws)

I want the API to stay explicit. Simply by reading the code, you must be able to follow what's going on. Introducing a callback-based API would break this design principle.

Since I'm not familiar with Tornado's implementation of websockets, I may have failed to understand your idea. If so, could you clarify it?

@songololo
Copy link
Author

Hi, for now I'm using the:

if message is None:
           break

and:

if not websocket.open:
            break

and they are doing the trick. I understand what you are saying re: callbacks and this makes sense. Thanks.

@aaugustin
Copy link
Member

Cool.

Let me know if you think of improvements that are compatible with this philosophy. The library is rather simple at this time. Perhaps it could support some common use cases better :-)

@songololo
Copy link
Author

The try / finally style works nicely for the send method.

It would be nice if something similar were possible with the receive method i.e. if recieve() could throw an error that allows for activating the 'graceful exit' code through error handling rather than checking for None types?

@aaugustin
Copy link
Member

Yes, I'm aware that this API creates some ugly code.

Unfortunately I can't change it without breaking backwards-compatibility in a way that will affect everyone :-/

@aaugustin aaugustin mentioned this issue Jun 23, 2019
18 tasks
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