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

Interactive Window creation and restore #10808

Closed
rebornix opened this issue Jul 14, 2022 · 3 comments
Closed

Interactive Window creation and restore #10808

rebornix opened this issue Jul 14, 2022 · 3 comments
Assignees
Labels
debt Code quality issues interactive-window Impacts interactive window

Comments

@rebornix
Copy link
Member

Follow up item of #10785 (comment)

Currently the interactive window is created in an asynchronous fashion (due to the async kernel detection), there is a chance of race condition (InteractiveWindowProvider._windows still empty when multiple clicks happen on Run Cell code lens). Supporting hot exit for Interactive Window also makes the workflow more complex as the hot exit / window restore happens synchronous at extension activation, before we have all notebook controllers ready.

A proposal coming from discussions in #10785 (comment) is

  • Create InteractiveWindow immediately with owner and mode, push it to _windows in both cases (create or restore)
  • Have one explicit start method to get the InteractiveWindow prepared for cell execution, which includes
    • If it's new IW
      • Get preferredController
      • Call interactive.open to create the IW editor in VS Code
      • Start kernel
    • If it's a restored IW, we then have the InteractiveTab
      • Reveal the tab into view (currently we use openNotebookDocument API, in the future we would have Tab API to archive this)
      • Get preferredController and set it as the active controller (maybe we don't need this as the notebook controller provider already tells VS Code which controller to use)
      • Start kernel
@rebornix rebornix added bug Issue identified by VS Code Team member as probable bug debt Code quality issues triage-needed Issue needs to be triaged and removed bug Issue identified by VS Code Team member as probable bug labels Jul 14, 2022
@DonJayamanne
Copy link
Contributor

We might want to consider the following scenario as well:

  • What happens when IW is restored and user attempts to run some code immediately in the textbox?
    • What kernel would be used at this point (given that our code to set the preferred controller has not yet executed)
  • What happens when IW is restored and user changes the kernel immediately (before we've managed to set the preferred controller).

In both cases, we could end up in a situation where kernel A is first selected, then our code runs and we change/update the preferred controller & things might not work as expected.
E.g. when restoring its not restored to the right kernel or we end up changing what the user selected.

Basically i'm highlighting the fact that there are still places where we could have race conditions and I'd prefer to avoid all of them completely this way, we don't run into any unknown or difficult to diagnose user bugs or failures on CI ...

@rchiodo rchiodo removed the triage-needed Issue needs to be triaged label Jul 14, 2022
@rchiodo
Copy link
Contributor

rchiodo commented Jul 14, 2022

@rebornix Should we have a meeting about this?

@rebornix
Copy link
Member Author

@rchiodo yes let's arrange one for this.

@amunger amunger modified the milestones: April 2023, May 2023 Apr 26, 2023
@amunger amunger modified the milestones: May 2023, June 2023 May 31, 2023
@amunger amunger closed this as completed Dec 6, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues interactive-window Impacts interactive window
Projects
None yet
Development

No branches or pull requests

4 participants