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.

Sign up for GitHub

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

Jump to bottom

Notebook interrupt button can be unresponsive #3805

Closed
ntluong95 opened this issue Jul 2, 2024 · 5 comments
Closed

Notebook interrupt button can be unresponsive #3805

ntluong95 opened this issue Jul 2, 2024 · 5 comments
Assignees
@seeM
Labels
area: notebooks Issues related to Notebooks category. bug Something isn't working investigate Needs initial, limited investigation to prioritize support
Milestone
2025.01.0 Pre-Rel...

Comments

@ntluong95
Copy link

Positron Version:

Positron Version: 2024.06.1 (system setup) build 2024.06.1-27
Code - OSS Version: 1.90.0
Commit: a893e5b
Date: 2024-06-26T01:33:58.809Z
Electron: 29.4.0
Chromium: 122.0.6261.156
Node.js: 20.9.0
V8: 12.2.281.27-electron.0
OS: Windows_NT x64 10.0.22621

Steps to reproduce the issue:

  1. Open a Jupyter Notebook
  2. Run a random cell

To be honest, I don't know how to reproduce the problem. It works fine for me before but not now

What did you expect to happen?

The cell run

Were there any error messages in the output or Developer Tools console?

The cell didn't run. As can be seen from the screenshot, it took 11 minutes and not yet finished package loading
image
@ntluong95 ntluong95 added the bug Something isn't working label Jul 2, 2024
@jthomasmock jthomasmock added area: notebooks Issues related to Notebooks category. labels Jul 2, 2024
@juliasilge
Copy link
Contributor

If this happens in a reproducible way for you (or even if something seems to make it happen more often), please do share that with us!

@juliasilge juliasilge added the investigate Needs initial, limited investigation to prioritize label Jul 8, 2024
@juliasilge juliasilge changed the title Jupyter notebook did not work Runtime/notebook integration may be causing stability problems in Jupyter notebooks Jul 8, 2024
@juliasilge juliasilge added this to the Release Candidate milestone Jul 8, 2024
@seeM
Copy link
Contributor

seeM commented Jul 10, 2024

One possible issue I can see from the code is that we aren't listening to and handling session.onDidEndSession in the NotebookSessionService for potential errors after startup.

@petetronic
Copy link
Collaborator

Something like this may also be behind posit-dev/ark#707?

@seeM
Copy link
Contributor

seeM commented Jul 11, 2024

@petetronic yes that sounds very possible.

@seeM seeM self-assigned this Jul 17, 2024
This was referenced Jul 25, 2024
Closed
Merged
testlabauto added a commit that referenced this issue Jul 25, 2024
@testlabauto
Notebook smoke tests robustness fixes (#4153)
9d85804 
This PR has some notebook test robustness fixes. The basic notebook test
now has two types of retries:
* retry entry of code in cell if entered text not visible post entry
* retry cell execution if output does not appear

The notebook variables test just has one minor fix: allow one second
post execution before beginning to look for variables.

Note that there is an open issue that might still lead to these tests
failing: #3805

### QA Notes

All smoke tests should pass
@seeM seeM modified the milestone: Release Candidate Jul 29, 2024
@seeM seeM removed their assignment Jul 29, 2024
@seeM seeM mentioned this issue Oct 15, 2024
Closed
@seeM seeM changed the title Runtime/notebook integration may be causing stability problems in Jupyter notebooks Notebook interrupt button can be unresponsive Nov 29, 2024
@seeM seeM mentioned this issue Dec 2, 2024
Merged
@seeM seeM self-assigned this Dec 2, 2024
seeM added a commit that referenced this issue Dec 5, 2024
@seeM
Use a global interrupt handler and handle interrupt edge cases (#5566)
f86d314 
Attempts to address #3805.

There are two parts to this:

1. Moved from using cell-level interrupt (`NotebookCellExecution.token`)
to notebook-level interrupt (`NotebookController.interruptHandler`),
which I believe is the correct approach when there is really a single
global kernel backing executions. Cell-level interrupt also only let you
interrupt once, and sometimes Python requires multiple interrupts.
2. Better handling for edge cases when interrupting such as the session
having exited unexpectedly.

### QA Notes

Since the addressed issue is intermittent, it's hard to test. Opening
and closing notebooks, and starting and interrupting executions in
different ways should all be more stable with this PR.

Integration tests should also pass locally.
@seeM seeM mentioned this issue Dec 5, 2024
Merged
seeM added a commit that referenced this issue Dec 5, 2024
@seeM
Remove the cell-level interrupt so that the notebook-level interrupt …
45ec5c2 
…works (#5629)

Related to #3805. I forgot to remove this in the previous PR.
@midleman
Copy link
Contributor

midleman commented Dec 6, 2024

Verified Fixed

Positron Version(s) : 2025.01.0-32
OS Version(s) : MacOS

Test scenario(s)

Confirmed that basic running and interrupting cells in the notebook works as expected. The real test of this fix will be whether it reduces the flakiness of our automated notebook-create tests. We’ve occasionally encountered issues where a simple cell execution hangs in CI, and hopefully this change fixes that.

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

n/a

@midleman midleman closed this as completed Dec 6, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@seeM seeM

Labels
area: notebooks Issues related to Notebooks category. bug Something isn't working investigate Needs initial, limited investigation to prioritize support
Projects
None yet
Milestone
2025.01.0 Pre-Release
Development

No branches or pull requests
6 participants
@seeM @petetronic @juliasilge @midleman @jthomasmock @ntluong95