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

[wip] Add Language Server Proxy Kernel #278

Open
wants to merge 87 commits into
base: main
Choose a base branch
from

Conversation

bollwyvl
Copy link
Collaborator

@bollwyvl bollwyvl commented Jun 7, 2020

Binder

References

Code changes

A lot. Too much. Enough that this might be unmergeable, as-is and will bitrot really fast. Here's the various yak shaves i had to go down:

  • add a language server kernel
    • now owns the LanguageServerManager
    • add a jupyter.lsp comm target to replace WebSockets (in data) and REST (in metadata)
      • jupyter.lsp.control comm target for status, etc. (formerly REST API)
        • also handles PageConfig
      • jupyter.lsp.language_server comm target for JSON-RPC (formerly WebSockets)
  • remove serverextension
  • remove lsp-ws-connection dependency and code
    • removes event-emitter dependency
  • remove runtime vscode-* dependencies
    • add type-only import of vscode-language-server-protocol (duplicate the one enum)
      • upgrade typescript
        • upgrade prettier
  • add a mostly-faithful reimplementation of connection and lsp-ws-connection
    • i might move this down one layer, so that we can Multiple sources of LSP messages on frontend and backend #184 better
    • add a LSP-over-Comm
      • add a JSON-RPC-over-Comm
      • add a single LSP type entrypoint, use throughout (e.g. lsProtocol -> LSP)
    • generate a base connection so the connection will support the full spec
    • better encapsulate capbilities
  • handle kernel restart
  • centralize all imported, reproduced, and derived LSP types and constants

User-facing changes

  • "finishing" installation would require:
    • jupyter lsp kernelspec install instead of
    • jupyter serverextension enable
  • A Language Server kernel will show up in the Notebooks and Console section
  • either we need to
    • hide it
    • brand it
  • a single Language Server Kernel is reconnected/started when lab starts (visible in Running)
    • everything waits until it has received the initial status and URIs

Backwards-incompatible changes

At once, total, as well as curiously backwards compatible. I really didn't touch the UI at all, aside from changing imports, and making a few more things asynchronous. But yeah, if anybody was working on a non-open fork and had been doing fancy stuff with our non-according-to-anybody's-spec websockets or REST API, they're hosed.

Chores

  • linted
  • tested
  • documented
  • changelog entry

Thoughts

I'm actually pretty happy with how this is coming together. In the long run, the Kernel Message Spec will be more stable over time than our custom stuff.

While there's a few singletons right now, I can imagine a kernel being able to register the comm targets, and our manager would be able to ask them if they'd like to contribute.

I'll be punching on this a bit more, but relatively soon, we'll have to decide a path forward: while there are a lot of commits, I don't know if i have the patience to rip this back apart into easily-reviewed pieces.

  • just take it into master, try a release against 2.1
    • kinda scary, and I don't want the burden to be on open PRs to get it working
  • start a next branch and target that
    • roll it out at some point in the future (e.g. post Lab 2.2)
    • handle incumbent PRs more piecewise

Stay tuned!

This was referenced Jul 28, 2020
@krassowski
Copy link
Member

I'm thinking about cherry-picking the improvements from packages/jupyterlab-lsp/src/lsp.ts and promoting it to protocol protocol package; I would then rework the current ws-connection to implement ILSPConnection as defined here and use the new protocol bits.

@bollwyvl
Copy link
Collaborator Author

bollwyvl commented Aug 31, 2020 via email

@bollwyvl
Copy link
Collaborator Author

bollwyvl commented Jan 9, 2021

I do intend to get back to this, but it will be some time before other stuff settles down, I'll wager. I can also mentor anybody that might interested in picking this back up.

Some thoughts I had captured elsewhere, but is relevant here:

  • This is still worth doing
    • As it is, it would free use from nbclassic vs notebook vs jupyter_server issues
      • we would still have to deal with them in test, but from an API perspective, not really (just the labextension file structure)
      • being a kernel has other nice benefits
        • you get a top-row launcher icon
          • at some point we need to support LSP Workspaces, maybe that's what an Untitled.lsp-workspace.ipynb is?
    • Having an interactive kernel-based proxy would mean one could use the visual debugger
      • i've had good luck demoing the current Lab-LSP implementation along with the debugger, as they are running on two entirely separate channels/loops
      • being able to pause/step an LSP connection 😘 👌

Some I think we need for this work to be successful/maintainable, long-term:

  • have a single source of type/data validation truth for (versioned) LSP messages in kernel-level comms, kernel-side listeners, etc.
    • i still think JSON schema is the right lowest-common denominator
      • it's fine if it's generated from typescript source... getting the enums right is probably the trickiest part

@krassowski
Copy link
Member

I am concerned about two things:

  • performance
  • changes in the kernel specs

The first one we can measure the other we don't have much influence on.

@krassowski
Copy link
Member

But I absolutely did not mean to close this one, there is a lot of valuable work in here :)

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

Successfully merging this pull request may close these issues.

Language Server Kernel
2 participants