Skip to content

Start supervisor concurrently with Positron server; address some state issues #7198

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

Draft
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

jmcphers
Copy link
Collaborator

@jmcphers jmcphers commented Apr 9, 2025

This change addresses some of the edge cases around the relationship between Positron Workbench sessions, browser tab sessions, extension host sessions, supervisor sessions, and R/Python sessions.

The primary change is the way the supervisor is started in Workbench. Formerly, it worked the same way it did on Desktop: every workspace started its own supervisor. Now:

  • The supervisor is started at the same time as the shared Node process
  • All of the workspaces in the Positron Workbench session share that supervisor

This improves performance, because the supervisor is already warm by the time we're ready to start a session. It also fixes some state issues that arise from storing the supervisor connection information in workspace state, which is too durable; on Workbench, the connection information now comes from a session-scoped file path.

The remainder of the change is around handling multiple tabs connecting to the same workspace. When this happens, you will now see a dialog explaining what happened, along with a message in the Console showing that your sessions have been transferred to the new window.

image

Positron should otherwise keep running fine in the old window if you have any non-data-related work to do, such as finishing up with script edits or Terminal tasks.

You might be wondering what happens if you start a new session in a tab after its existing sessions get moved to another tab. The system is designed to handle this; you can perform this move to get two tabs with different sets of sessions going against one workspace if you really want/need to. Opening a new tab (or reloading an existing one) will cause it to adopt all the sessions from any other tabs/windows, by design, so that you can't orphan any sessions.

Addresses #5374
Addresses #6174
Addresses #6799
Addresses #7157
Partially addresses #7078 (workbench only)

Release Notes

New Features

  • N/A

Bug Fixes

QA Notes

This is a high-risk change because it affects how we start and reconnect to sessions, which involves a lot of different pieces (see list of "sessions" above) plus a lot of different state stored in a lot of different places.

Things that deserve extra attention:

  • moving sessions to and from different tabs
  • switching workspaces and windows
  • reloading desktop & browser tabs
  • switching browsers (note that console contents will not be preserved when you do this outside Workbench, since it is stored in the browser in that configuraiton)
  • reloading/restarting kernels

Also note that in web mode, we can no longer respect the supervisor log level setting or the idle shutdown hours setting -- there is no (reasonable) way for us to read these settings at the time the supervisor is started. The latter setting doesn't make sense in web mode anyway; if we need to adjust the log level in web mode we will need to build another way to do it.

Test tags: @:sessions

@jmcphers jmcphers changed the title Bugfix/session persistence scoping Start supervisor concurrently with Positron server; address some state issues Apr 9, 2025
Copy link

github-actions bot commented Apr 9, 2025

E2E Tests 🚀
This PR will run tests tagged with: @:critical @:sessions

readme  valid tags

@jmcphers jmcphers requested a review from sharon-wang April 9, 2025 19:00
@petetronic petetronic marked this pull request as draft April 16, 2025 17:34
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.

1 participant