Add web sockets support #128
Conversation
Using 'Autobahn' with 'twisted' to provied Web Sockets
There was a problem hiding this comment.
Hey @npradeep357, thanks a lot for your contribution! My main comment for now is it's possible to add some tests for this new functionality, given that right now I see none.
Besides that, I'd like to ask @krassowski what he thinks about this one.
|
@ccordoba12, Thank you, will add tests for this. Also, regarding the checks, there are Linux, Mac and Windows tests failure which says the version mentioned is not available for Py 3.6. I took the latest version of autobahn from PyPI which is 21.11.1. But the log in the tests shows only until 21.2.1 which is an older version released in Feb 2021. ref: autobahn PyPI history Suggest me if I have to move to older version or is there something I'm missing for Py 3.6? |
|
After some digging, I found out that autobahn is only supported from Python 3.7+. Is there a reason that PYLSP is supported from Python 3.6+? Can we move PYLSP to Python 3.7+? or should I downgrade the autobahn library which I used? |
There is because Python 3.6 is still a supported Python version. However, since it's going to reach end-of-life this month, we can drop support for it now. Please do that in a different pull request though, i.e. increase the Python version in our |
|
Hey @ccordoba12, I have created a separate PR for version change as mentioned. Can you review and approve #130 when you find some time. Thank you |
- renamed extras - autobahn-twisted to websockets - added more info on the import error
|
@krassowski made the suggested changes. |
|
Thanks! Looks fine by me, only minor comments now. |
There was a problem hiding this comment.
@npradeep357, thanks for your patience with this! Last review and then this should be ready (we can take a look at adding tests later).
@ccordoba12 I'm planning to change the websockets library to reduce 3rd party library footprint. If it can wait a week or two, I will update the PR. |
|
Sure, no problem with that. |
* Added websockets based implementation to reduce libraries and 3rd party dependencies * Removed message encode and decode as they are no longer needed Co-authored-by: Pradeep Neerukonda <pneeruko@opentext.com>
|
@ccordoba12, @krassowski, removed autobahn and twisted libraries and using websockets library to reduce libraries footprint. I have tested it my application and everything's working as expected.. websockets: https://websockets.readthedocs.io/en/stable/ |
|
@ccordoba12 @krassowski could you please review the latest changes. |
…rver into python-lsp-develop # Conflicts: # setup.cfg
|
@ccordoba12, can we merge this? |
There was a problem hiding this comment.
@npradeep357, thanks for your patience with this! There are still two unaddressed suggestions from my previous review. After you apply them, I'll merge your PR.
|
Hi @ccordoba12, Somehow these suggestions got overlooked. I have addressed them now. Thank you |
This pull request will enable PYLSP to run in Web Sockets configuration and can handle multiple connections in parallel.
The Idea was to handle
multipleparallel editing sessions using single process. This is achieved by creating new instances of PYLSPclassObject for each WS connection in a single server process.Fixes #117
As of now this PR only supports text based messaging over WS. Binary messaging support will be added in future.
This PR uses
autobahn websockets with twisted frameworkWebsockets LibraryThere is asyncio based framework but twisted is used because PYLSP is based on callbacks and asyncio is not compatible with it.referred docs:
https://autobahn.readthedocs.io/en/latest/websocket/programming.htmlhttps://autobahn.readthedocs.io/en/latest/installation.html
https://websockets.readthedocs.io/en/stable/
related dependencies are added to
setup.pysetup.cfgrelated usage documentation is added to README