-
Notifications
You must be signed in to change notification settings - Fork 18
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
Allow clients to connect over websocket #55
Comments
The go language server just does it's communication over plain tcp. That might actually be better than websockets. |
Some googling shows. TCP connections are nice but cannot be accessed from Webbrowsers so it might make sense to just implement both. |
github.com/gorilla/websocket seems to be the sanest go websocket lib around. |
This is mostly implemented here: https://github.com/prometheus-community/promql-langserver/blob/master/langserver/websocket.go However this code still is still untested, so this issue won't be closed until this functionality is verified to work. |
Update: There exists the experimental websocket package for this now. |
similar implementation in rest package would be implemented with the websocket package right ? |
Regarding metrics? The main difference is that http requests are upgraded to websocket connections that are kept open as long as the client is running. |
not regarding metrics. i mean would the websocket protocol also expose similar endpoint like hover, completion etc. as exposed in rest(using http) package? |
No, it opens one endpoint that upgrades to a websocket connection that serves as a transport layer for the Language Server Protocol. The rest package is a alternative API for use cases where the LSP would be to complex, but only works for single queries which are usually small enough for HTTP GET requests (i.e. YAML files are intentionally not supported). For an extensive discussion, see prometheus/prometheus#6160 . |
clear now. Thanks. |
Yes. |
The websocket module was never finished and due to API changes in various places, doesn't work in it's current form. Also it's incompatible with #181. If someone wants to solve #55, it's better to start over. Signed-off-by: Tobias Guggenmos <[email protected]>
No description provided.
The text was updated successfully, but these errors were encountered: