Add socket to environ to allow user sending websocket messages#470
Add socket to environ to allow user sending websocket messages#470smeng9 wants to merge 2 commits intoPylons:mainfrom
Conversation
|
Could you add tests to cover this? |
|
Hi @kgaughan Tests added! |
|
Hi @kgaughan , Any chance this can be reviewed soon? |
|
How does the hand-off back to Waitress happen? This completely breaks the way that waitress handles sockets/WSGI communication. I don't think this is a good idea, nor one that the Pylons Project would like to support. This will tie up the thread for the entire time the WebSocket is alive, which is just a bad idea. Waitress was never built with this in mind. The test you added is also not an appropriate test, it just adds a new socket to the WSGI environment, not the one that is passed in, does not show the upgrade process or making sure that that this won't break anything else. |
|
Let me explain how this will work. Currently waitress does not support websocket due to not exposing the raw socket object to request handlers. We use a very popular library called simple-websocket, and we want it to be able to access the socket object and handle the communication (the upgrade process) for us. https://github.com/miguelgrinberg/simple-websocket/blob/main/src/simple_websocket/ws.py#L248 . As you can see, if we want to handle the long running communication, there has to be some thread running. Waitress supports multi threading, and it is user's responsibility to add more thread. Also, for the added extra property If you were to introduce the websocket to your project, what would be your recommended approach? |
|
It has been a while since this merge request is opened. I have also did some research how websocket should be supported through wsgi. Mainly the socket objects needs to be passed as custom extensions to the request https://stackoverflow.com/questions/75538581/does-pythons-wsgi-specification-have-anything-to-say-about-websockets Other mainstream wsgi handlers already added the support, but they are only available on Linux platform. As waitress natively supports Windows, I hope we can push this forward a bit. Thanks! |
No description provided.