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

Use new Jupyter supervisor by default in desktop mode #5262

Closed
jmcphers opened this issue Nov 4, 2024 · 1 comment
Closed

Use new Jupyter supervisor by default in desktop mode #5262

jmcphers opened this issue Nov 4, 2024 · 1 comment
Labels
area: core Issues related to Core category. area: kernels Issues related to Jupyter kernels and LSP servers

Comments

@jmcphers
Copy link
Collaborator

jmcphers commented Nov 4, 2024

Currently, the experimental Jupyter kernel supervisor ("Kallichore") is used as the default in Positron Workbench, but we use a totally different supervisor (the "Jupyter Adapter") by default in Positron Desktop. We should make the new supervisor the default in Positron Desktop, too; it is expensive to maintain two different supervisors, and the old supervisor has some known issues we don't plan to fix.

Details:

  • The old supervisor should remain in place for awhile so we can have folks revert to it for debugging purposes (i.e. if they are seeing a problem, we need to be able to tell if it is caused by the new supervisor or is unrelated.)
  • Logs for the new supervisor need to be moved out of a terminal and into an Output tab so they can be collected by desktop users for issue reporting.
  • The codename "Kallichore" needs to be eliminated (in settings, etc.) now that the supervisor is mature enough to be the default. We should just call it the "Jupyter supervisor" or similar in the UI, and may also want to rename the extension itself for clarity.
@jmcphers jmcphers added area: kernels Issues related to Jupyter kernels and LSP servers area: core Issues related to Core category. labels Nov 4, 2024
@jmcphers jmcphers added this to the 2024.12.0 Pre-Release milestone Nov 4, 2024
jmcphers added a commit that referenced this issue Nov 8, 2024
…5299)

This change brings Kallichore out of experimental mode and makes it the
default for all environments (server and desktop).

<img width="430" alt="image"
src="https://github.com/user-attachments/assets/4809ca1b-2e87-473d-963e-a7d49a8bd416">

It also rebrands the supervisor, removing its codename and referring to
it instead as the "Positron Kernel Supervisor". The extension has also
been renamed, from `kallichore-adapter` to `positron-supervisor` to
group it with our other extensions and make the name more meaningful.

It contains some improvements to logging; formerly you had to let
Kallichore run in a visible terminal to see its logs. Now its logs are
also streamed to the "Positron Kernel Supervisor" output channel. (This
doesn't increase the number of output channels, as this channel replaces
the "Kallichore Adapter" channel.)

Finally, it includes a fix for a bug that could cause cell execution
hangs when switching notebook kernels rapidly under the new supervisor.
This issue was found via an automated test; the fix is to clean up some
references to the notebook's interpreter session as soon as we know the
kernel has exited (whether via an exit state change or exit event).

The change does _not_ remove the Jupyter Adapter. The Jupyter Adapter
remains, for now, as a troubleshooting tool (i.e. if we're seeing any
unexpected behavior and suspect it is caused by the supervisor, we can
switch back to the adapter to compare notes). My hope is to have the
supervisor as the default in the 2024.12 release, and to be able to
remove the Jupyter Adapter entirely in 2025.01 or 2025.02.

Addresses #5262.

### QA Notes

- If you find any issues, please include lines from the new Positron
Kernel Supervisor output channel that seem relevant.
- This change passes the full suite of Electron tests.
https://github.com/posit-dev/positron/actions/runs/11714219722/attempts/1
- This change should not break Reticulate, because Reticulate always
uses the Jupyter Adapter. Positron can mix and match supervisors, so
it's fine for R/Python to use Kallichore even when Reticulate is using
the Adapter.
- I'm hopeful this will address some of the "socket disposed" / "address
in use" errors we sometimes see in tests, since Kallichore has some code
that tries to reduce the odds of these things happening. However, it is
difficult to completely eliminate this problem since it's due to a race
condition that is built into the protocol. See
posit-dev/kallichore#2 for more notes and
planned work.

---------

Signed-off-by: Jonathan <jonathan@posit.co>
Co-authored-by: sharon <sharon-wang@users.noreply.github.com>
@testlabauto
Copy link
Contributor

Verified Fixed

Positron Version(s) : 2024.12.0-23
OS Version          : ubuntu, osx, windows

Test scenario(s)

Looking good locally and in automation runs

Link(s) to TestRail test cases run or created:

jmcphers added a commit that referenced this issue Nov 19, 2024
…5299)

This change brings Kallichore out of experimental mode and makes it the
default for all environments (server and desktop).

<img width="430" alt="image"
src="https://github.com/user-attachments/assets/4809ca1b-2e87-473d-963e-a7d49a8bd416">

It also rebrands the supervisor, removing its codename and referring to
it instead as the "Positron Kernel Supervisor". The extension has also
been renamed, from `kallichore-adapter` to `positron-supervisor` to
group it with our other extensions and make the name more meaningful.

It contains some improvements to logging; formerly you had to let
Kallichore run in a visible terminal to see its logs. Now its logs are
also streamed to the "Positron Kernel Supervisor" output channel. (This
doesn't increase the number of output channels, as this channel replaces
the "Kallichore Adapter" channel.)

Finally, it includes a fix for a bug that could cause cell execution
hangs when switching notebook kernels rapidly under the new supervisor.
This issue was found via an automated test; the fix is to clean up some
references to the notebook's interpreter session as soon as we know the
kernel has exited (whether via an exit state change or exit event).

The change does _not_ remove the Jupyter Adapter. The Jupyter Adapter
remains, for now, as a troubleshooting tool (i.e. if we're seeing any
unexpected behavior and suspect it is caused by the supervisor, we can
switch back to the adapter to compare notes). My hope is to have the
supervisor as the default in the 2024.12 release, and to be able to
remove the Jupyter Adapter entirely in 2025.01 or 2025.02.

Addresses #5262.

- If you find any issues, please include lines from the new Positron
Kernel Supervisor output channel that seem relevant.
- This change passes the full suite of Electron tests.
https://github.com/posit-dev/positron/actions/runs/11714219722/attempts/1
- This change should not break Reticulate, because Reticulate always
uses the Jupyter Adapter. Positron can mix and match supervisors, so
it's fine for R/Python to use Kallichore even when Reticulate is using
the Adapter.
- I'm hopeful this will address some of the "socket disposed" / "address
in use" errors we sometimes see in tests, since Kallichore has some code
that tries to reduce the odds of these things happening. However, it is
difficult to completely eliminate this problem since it's due to a race
condition that is built into the protocol. See
posit-dev/kallichore#2 for more notes and
planned work.

---------

Signed-off-by: Jonathan <jonathan@posit.co>
Co-authored-by: sharon <sharon-wang@users.noreply.github.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: core Issues related to Core category. area: kernels Issues related to Jupyter kernels and LSP servers
Projects
None yet
Development

No branches or pull requests

2 participants