-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Fix interrupt to support a timeout and handle crashes #3614
Conversation
Fix problem with cancel in jupyterExecution with cached promise
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.catch(err => { | ||
status.dispose(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we need to notify the user? #Resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (this.specFile && | ||
IsGuidRegEx.test(path.basename(path.dirname(this.specFile)))) { | ||
try { | ||
fs.removeSync(path.dirname(this.specFile)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make this async. #Resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although I could just ignore errors on it.
In reply to: 240369636 [](ancestors = 240369636,240364388)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can't be. The dispose can't return anything.
A number of async dispose
methods already exist in DS code, how is this any different?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We must always avoid sync operations. Specially here, perfoming I/O operations in dispose slows down overall dispose method time.
For #3511
Also did some work around making sure cancel behaves correctly. Cancelling a connection happens on exporting a file and running all cells. There was a bug where the cancel token from one request was being reused in another.
package-lock.json
has been regenerated by runningnpm install
(if dependencies have changed)