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

Allow clients to connect over websocket #55

Open
slrtbtfs opened this issue Nov 6, 2019 · 11 comments
Open

Allow clients to connect over websocket #55

slrtbtfs opened this issue Nov 6, 2019 · 11 comments
Labels
enhancement New feature or request

Comments

@slrtbtfs
Copy link
Member

slrtbtfs commented Nov 6, 2019

No description provided.

@slrtbtfs slrtbtfs added enhancement New feature or request REST API Concerns the REST API labels Nov 6, 2019
@slrtbtfs
Copy link
Member Author

The go language server just does it's communication over plain tcp. That might actually be better than websockets.

@slrtbtfs
Copy link
Member Author

Some googling shows. TCP connections are nice but cannot be accessed from Webbrowsers so it might make sense to just implement both.

@slrtbtfs
Copy link
Member Author

github.com/gorilla/websocket seems to be the sanest go websocket lib around.

@slrtbtfs
Copy link
Member Author

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.

@slrtbtfs slrtbtfs removed the REST API Concerns the REST API label Feb 28, 2020
@slrtbtfs
Copy link
Member Author

Update: There exists the experimental websocket package for this now.

@haibeey
Copy link
Contributor

haibeey commented Mar 15, 2020

similar implementation in rest package would be implemented with the websocket package right ?

@slrtbtfs
Copy link
Member Author

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.

@haibeey
Copy link
Contributor

haibeey commented Mar 15, 2020

not regarding metrics. i mean would the websocket protocol also expose similar endpoint like hover, completion etc. as exposed in rest(using http) package?

@slrtbtfs
Copy link
Member Author

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 .

@haibeey
Copy link
Contributor

haibeey commented Mar 17, 2020

clear now. Thanks.
seems like the remaining checklist for this to close is testing the websocket module?

@slrtbtfs
Copy link
Member Author

Yes.

slrtbtfs added a commit that referenced this issue Jul 7, 2020
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants