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

Support picking IJupyterServerUriInfo in command jupyter.selectjupyteruri #9683

Closed
rebornix opened this issue Apr 14, 2022 · 6 comments
Closed
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug notebook-remote Applies to remote Jupyter Servers verified Verification succeeded
Milestone

Comments

@rebornix
Copy link
Member

As part of the lazy kernel exploration microsoft/vscode#146942 (comment) , we want to support IJupyterServerUriInfo in command jupyter.selectjupyteruri. With this support

  • Extensions can register their own remote server provider through api.registerRemoteServerProvider
  • Extensions then ask Jupyter to switch to their remote server without showing the Remote Server quick pick
@rebornix rebornix added bug Issue identified by VS Code Team member as probable bug and removed bug Issue identified by VS Code Team member as probable bug labels Apr 14, 2022
@rebornix rebornix added this to the April 2022 milestone Apr 14, 2022
@rchiodo rchiodo removed their assignment Apr 14, 2022
@DonJayamanne
Copy link
Contributor

This is related to the same work we need for Azure ML compute work. https://github.com/microsoft/vscode-jupyter-internal/issues/270

@IanMatthewHuff IanMatthewHuff removed their assignment Apr 14, 2022
@greazer greazer added bug Issue identified by VS Code Team member as probable bug iteration-candidate notebook-remote Applies to remote Jupyter Servers and removed needs-triage labels Apr 14, 2022
@greazer greazer removed this from the April 2022 milestone Apr 14, 2022
@DonJayamanne DonJayamanne self-assigned this May 9, 2022
@DonJayamanne DonJayamanne added this to the May 2022 milestone May 9, 2022
@DonJayamanne
Copy link
Contributor

DonJayamanne commented May 11, 2022

Details 1
  • Assume we run a cell, reload VS Code, now we have an entry in the cache for an active kernel with id xyz that points to a IJupyterServerUri with id = a and handle handle1
  • Assume we shutdown VS Code
    • The extension that provides the handle handle1 has now shutdown the jupyter server, hence we know the active kernel xyz will no longer exist.
  • Reload VS Code
    • The 3rd party extension will now return IJupyterServerUri with id = 'a' and handle handle2.
    • Jupyter Extension still lists the old kernel xyz

Questions:

  • Should we remove the entry for old active kernel xyz? I don't think we should, we should leave this so user knows that it is no longer available (required as it solves issue Adjust error notification behavior when unable to connect to the remote jupyter server #9167)
  • If user runs a cell against this, what should the user do?
    • There's no point providing the option to remove the old server as its no longer available either.
    • Should we instead keep the old server in the list as well? I.e. should we have an entry for the server with the handle handle1 and handle2, if yes, then display names MUST be unique. But how do we guarantee this, they are created by 3rd party extensions.
    • Should we instead just let the user know that the server is no longer available and provide an option to just select another kernel (this feels like the most valid option)
Multiple Remotes (dead kernels)

What if we have a cached active kernel and the 3rd party extension has been uninstalled?
At this point, the only way to unblock the user is to display the error and provide the option to Forget the conection. Managing connections might not work as it might not even exist anymore (as the extension isn't registering with us anymore)

To get things working, we'd need to keep track of all of the valid Ids & handles.
We can put this into global memento, (generate the serverId from this and store that into a global mento), when extnsion loads we know what's valid. If user choses to forget a Uri, we remvoe that entry from the memento.

This way, we can remove items from teh cache easily by filtering them based on what's in the global memento (which won't require any awaits or the like).

3rd party extensions

When displaying errors about a remote jupyter server. Should we let the user know what extension should be looked at.
Today we just say one fo the extension may have been uninstalled, Should we instead let them know which extension, we can do this today (pretty easily - we have a cache of this).

@rchiodo
Copy link
Contributor

rchiodo commented May 11, 2022

Reload VS Code
The 3rd party extension will now return IJupyterServerUri with id = 'a' and handle handle2.
Jupyter Extension still lists the old kernel xyz

AFAIK it's not working like this today. The handle is stored by us in the encrypted storage. We always give it to the 3rd party extension to provide us with a URI. So they can generate a new URI if they want.

@rchiodo
Copy link
Contributor

rchiodo commented May 11, 2022

Additionally I believe we use that handle to find the extension to activate in order to ask them for a new URI.

@DonJayamanne
Copy link
Contributor

We always give it to the 3rd party extension to provide us with a URI. So they can generate a new URI if they want.
Yes thats correct, but the implementation is upto the 3rd party extension and we don't assume that its the same nor different.
My scenario here is that the old handle is no longer valid and the 3rd party returns handle2 and not handle1 anymore. In which case, we need to handle that scenario

use that handle to find the extension to activate in order to ask them for a new URI.

We already have code in place to handle that, we have code that calls getServerUri where we pass in the handle.

@DonJayamanne
Copy link
Contributor

Extensions then ask Jupyter to switch to their remote server without showing the Remote Server quick pick

The use of the command isn't recommended.
We now have an API exposed for this
addRemoteJupyterServer(providerId: string, handle: JupyterServerUriHandle): Promise<void>;

@rebornix rebornix added the verified Verification succeeded label Jun 2, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug notebook-remote Applies to remote Jupyter Servers verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

5 participants