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

Proposal: Streamline retrieving sessions via the extension API #2620

Open
seeM opened this issue Apr 3, 2024 · 1 comment
Open

Proposal: Streamline retrieving sessions via the extension API #2620

seeM opened this issue Apr 3, 2024 · 1 comment
Labels
area: api Issues related to API category. area: runtimes Issues related to Language Runtimes

Comments

@seeM
Copy link
Contributor

seeM commented Apr 3, 2024

Our extensions often retrieve sessions with some given metadata. Currently, each extension maintains a list of its known sessions to facilitate this. This is unnecessarily verbose and error-prone. We should instead consider expanding the API to streamline this.

Here's why we currently retrieve sessions across our extensions:

  1. Get the session corresponding to a given notebook URI.
  2. Get the most recently created R console session that is not uninitialized or exited.
  3. Check if a Python session exists with a given interpreter path.
  4. Get the Python session with a given interpreter path.

Each of these ultimately filters the entire list of running sessions based on session metadata or session state (note that a session's state is not currently exposed in the API).

Proposal:

  1. Expose positron.runtime.getSessions(), returning all running sessions i.e. the union of values of the _consoleSessionsByLanguageId and _notebookSessionsByNotebookUri maps, and let callers do their own filtering.

    I'm not sure if there are performance concerns around sending the entire list of sessions on each call rather than adding filtering args to the function but from my understanding it should not be a problem.

  2. Callers can also use getSessions() to check for the existence of a session with the given properties, rather than additionally exposing hasXSession() functions too. From my understanding, this also wouldn't be a performance concern.

  3. For retrievals which are meaningful across extensions, expose more specific functions to ensure that all extensions use the same semantics e.g. getConsoleSession(languageId) (to be used in Positron R here). There's already a IRuntimeSessionService.getConsoleSessionForLanguage that we could wire this up to.

  4. Add a readonly state property to ILanguageRuntimeSession so that callers can filter on runtime session state too.

@seeM seeM added the area: api Issues related to API category. label Apr 3, 2024
@seeM seeM self-assigned this Apr 3, 2024
@seeM seeM changed the title Proposal: Improvements to the Positron API Proposal: Improvements to the Positron Extension API Apr 3, 2024
@seeM seeM changed the title Proposal: Improvements to the Positron Extension API Proposal: Streamline retrieving sessions via the extension API Apr 3, 2024
@seeM seeM removed their assignment Apr 4, 2024
@jmcphers
Copy link
Collaborator

jmcphers commented Apr 5, 2024

I like this proposal. The fact that the same internal session management tools have sprung up in multiple extensions signals that there's a need for this.

I'm not sure if there are performance concerns around sending the entire list of sessions

Definitely not. The set of sessions is never going to be large; even a complicated workspace with a bunch of notebooks open and interpreters running would be unlikely to exceed a dozen.

One thing to remember (especially for the R case and likely for other language packs) is that it's not enough to return a Positron session -- the extensions need to get the exact session object they registered, e.g. not an ILanguageRuntimeSession but specifically an RSession because we are retrieving the R session in order to do something R specific with it that isn't part of the lowest-common-denominator session interface.

@seeM seeM added this to the Release Candidate milestone Apr 5, 2024
@seeM seeM self-assigned this Nov 26, 2024
@seeM seeM added the area: runtimes Issues related to Language Runtimes label Nov 26, 2024
@seeM seeM removed their assignment Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: api Issues related to API category. area: runtimes Issues related to Language Runtimes
Projects
None yet
Development

No branches or pull requests

2 participants