microsoft / vscode-jupyter

VS Code Jupyter extension
https://marketplace.visualstudio.com/items?itemName=ms-toolsai.jupyter
MIT License
1.26k stars 276 forks source link

Explore how to simplify the code for managing connections to local and remote Jupyter Servers #12832

Closed DonJayamanne closed 9 months ago

DonJayamanne commented 1 year ago
### Tasks
- [ ] https://github.com/microsoft/vscode-jupyter/issues/13610
- [ ] https://github.com/microsoft/vscode-jupyter/issues/13578
- [ ] https://github.com/microsoft/vscode-jupyter/issues/13577
- [ ] https://github.com/microsoft/vscode-jupyter/issues/13631
- [ ] https://github.com/microsoft/vscode-jupyter/issues/13632
- [ ] https://github.com/microsoft/vscode-jupyter/issues/13633
- [ ] https://github.com/microsoft/vscode-jupyter/issues/13488
- [ ] https://github.com/microsoft/vscode-jupyter/issues/13345
- [ ] https://github.com/microsoft/vscode-jupyter/issues/13112
- [ ] https://github.com/microsoft/vscode-jupyter/issues/13015
- [ ] https://github.com/microsoft/vscode-jupyter/issues/8134
- [ ] https://github.com/microsoft/vscode-jupyter/issues/11645
- [ ] https://github.com/microsoft/vscode-jupyter/issues/13300
- [ ] https://github.com/microsoft/vscode-jupyter/issues/13634
- [ ] https://github.com/microsoft/vscode-jupyter/issues/13635
- [ ] https://github.com/microsoft/vscode-jupyter/issues/13637
- [ ] https://github.com/microsoft/vscode-jupyter/issues/13808
DonJayamanne commented 1 year ago

Problems

Ambiguous Uri, Url, ServerId #13540 * The term `Uri` is used in a number of places to point to the address of the Jupyter Server as well as an identifyer for a Jupyter Server. * Without inspecting the Uri we have no idea what it is, hence we have `if` conditions in a number of places * Unfortunately there are places where this `if` condition isn't used and has caused bugs (password storage and connection) * Error messages and the like assume the Url is the Jupyter Server Uri, but its not. * Basically its just too complex * The `Uri` is constructed from the Jupyter Provider Id and Handle, however without the extensionId it is not unique. * Its possible to have two extensions with the same provider id, and this can cause issues when we try to connect to them. * The `Uri` is constructed from the Jupyter Provider Id and Handle * Hence Uri is kind of always unique (provided we include the extension id into the mix) * Thus we do not need `computeServerId` * We have `serverId` used as identifiers in a number of places, along with this `Uri` as well * We have two types of identifiers Suggested Solutions: * [ ] Remove `serverId` * [ ] Replace `serverId` with the `Uri` * [ ] Replace Uri with an object, today its called `idAndHandle`, its basically a Jupyter Server Identifier (lets call this `JupyterServerProviderHandle` and will include `extensionId` as a new property
Invalid password not cleared until we start VS Code Root cause is the same as the previous problem. Uri !== Uri
Server already exists When attempting to add a new Server sometimes we end up with the error indicating this already exists. The problem is basically related to the previous issue as well as tech debt. Root cause is not yet known, but the code is a little too complex and we have too much state (instead of getting the latest state from memento/auth storage). It seems the in-memory state is stale and users run into this issue. Suggestions: * [ ] Remove the cache of `_servers` or only leave that when adding a new server and then remove that. I.e. where possible remove the caches.
No proper connection validation stage when starting a remote kernel When connecting to a server (or starting a kernel), this is what we do: * Create the connection `IJupyterConnection` * Next, start the session with the SessionManager using the IJupyterConnection It is at this point that the connection is validated. Problem: * If the password is invalid, we validate that when starting the session, not when creating the connection * If some other information is invalid then the same as above Question: * Should we have a separate step when we verify/validate the connection * Today we have a separate validation step only when selecting a kernel from a provider (e.g. when adding a Remote Jupyter Uri from the existing kernel Picker) Suggested Solutions: * [ ] Have a separate step to validate the connection The benefits are that we can easily update the code to incorporate management of the connection, updating the password, etc. Else adding all of this into the session creation code/layer seems messy.
Debt: Not waiting for async Session disposal to complete In JupyterConnection.ts when we dispose the session, we do not wait for it to finish after we've validated the connection information. Method `validateRemoteUri`, code `sessionManager.dispose().catch(noop);`
amunger commented 9 months ago

removing from verification queue