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

Do not activate Python before opening nb #6201

Merged
merged 1 commit into from
Jun 8, 2021
Merged

Conversation

DonJayamanne
Copy link
Contributor

For #6185

@DonJayamanne DonJayamanne requested a review from a team as a code owner June 8, 2021 16:30

if (this.isLocalLaunch) {
// Pre-warm fetching local kernels, for remote connections fetch as and when needed.
this.getNotebookControllers().catch(traceError);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how this slows things down? Is it the python extension activation?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it feels like we need to do this so that running a cell doesn't take forever. I'm wondering how this slows down the open as it should be a promise that's going to at least going to go off and look on disk for kernelspecs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes,
This causes pyton extension actiavtion, hence slows down extenison host and opening of the notebook.
We alraedy have code that gets this information after we get the notebook opened event.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

t should be a promise that's going to at least going to go off and look on disk for kern

Python extnesion spins up processes (process spawning is a sync process), all of this and other code (such as loading python extension js is synchronous), that blocks the extenssion host hence slows the current process of opening a notebook.

@DonJayamanne DonJayamanne merged commit 2001bcd into main Jun 8, 2021
@DonJayamanne DonJayamanne deleted the issueissue6185 branch June 8, 2021 17:54
DavidKutu pushed a commit that referenced this pull request Jun 10, 2021
* release candidate (#6059)

* release candidate

* Memorial Day API changes (#6056)

* update changelog

Co-authored-by: Ian Huff <ianhu@microsoft.com>

* Memorial Day API changes (#6056)

* Send only error type in reason (#6067)

* Fallback for sys.prefix not being returned by Python extension (#6053)

* Fixes to toggling output (#6068)

* Better telemetry when we don't find a matching kernel (#6060)

* Better data when fail to find kernel connection (#6070)

* change version and API port (#6085)

* change version to 2021.6.x

* match packagejson and changelog versions

* 6/2 API Changes (#6089)

Co-authored-by: Ian Huff <ianhu@microsoft.com>

* Port #6025 to release (#6103)

* Remove our Run Above/Below commands (#6026)

* Update changelog

* Fix pervasive test issue with editor properties (#6100) (#6106)

* update version to 2021.6.99 to be able to filter (#6122)

* update version to 2021.6.99 to be able to filter
update changelog

* update package lock and VSCode api

* don

* remove api changes in the code

* Port Don's fixes (#6115)

* Fix test failures resulting from VSCodes Notebook Start page (#6111)

* Disable kernel auto startup in untrusted workspace (#6088)

* Disable kernel auto startup in untrusted workspace

* Fixes

* Misc

* oops

* misc

* Fixes to breaking tests (#6074)

Co-authored-by: Don Jayamanne <don.jayamanne@outlook.com>

* Breaking changes ports to release (2 commits) (#6140)

* add component governance file (#6166)

* add component governance file

* remove npm components

* Cherry pick changes from main branch into release (#6174)

* final update (#6176)

* final update

* update changelog

* add missing thanks to changelog (#6197)

* Skip uploading vsix to azure blob store (#6204)

* publish release

* publish release

* remove last 2 numbers from release (#6219)

* publish release

* fix conflicts

* update engine to 1.58.0-insider

* return engine version to 1.57.0-insider

* Do not activate Python before opening nb (#6201) (#6230)

* publish release

Co-authored-by: Ian Huff <ianhu@microsoft.com>
Co-authored-by: Don Jayamanne <don.jayamanne@yahoo.com>
Co-authored-by: Don Jayamanne <don.jayamanne@outlook.com>
Co-authored-by: Joyce Er <joyceerhuiling@gmail.com>
DavidKutu pushed a commit that referenced this pull request Jun 16, 2021
* release candidate (#6059)

* release candidate

* Memorial Day API changes (#6056)

* update changelog

Co-authored-by: Ian Huff <ianhu@microsoft.com>

* Memorial Day API changes (#6056)

* Send only error type in reason (#6067)

* Fallback for sys.prefix not being returned by Python extension (#6053)

* Fixes to toggling output (#6068)

* Better telemetry when we don't find a matching kernel (#6060)

* Better data when fail to find kernel connection (#6070)

* change version and API port (#6085)

* change version to 2021.6.x

* match packagejson and changelog versions

* 6/2 API Changes (#6089)

Co-authored-by: Ian Huff <ianhu@microsoft.com>

* Port #6025 to release (#6103)

* Remove our Run Above/Below commands (#6026)

* Update changelog

* Fix pervasive test issue with editor properties (#6100) (#6106)

* update version to 2021.6.99 to be able to filter (#6122)

* update version to 2021.6.99 to be able to filter
update changelog

* update package lock and VSCode api

* don

* remove api changes in the code

* Port Don's fixes (#6115)

* Fix test failures resulting from VSCodes Notebook Start page (#6111)

* Disable kernel auto startup in untrusted workspace (#6088)

* Disable kernel auto startup in untrusted workspace

* Fixes

* Misc

* oops

* misc

* Fixes to breaking tests (#6074)

Co-authored-by: Don Jayamanne <don.jayamanne@outlook.com>

* Breaking changes ports to release (2 commits) (#6140)

* add component governance file (#6166)

* add component governance file

* remove npm components

* Cherry pick changes from main branch into release (#6174)

* final update (#6176)

* final update

* update changelog

* add missing thanks to changelog (#6197)

* Skip uploading vsix to azure blob store (#6204)

* publish release

* publish release

* remove last 2 numbers from release (#6219)

* publish release

* Do not activate Python before opening nb (#6201) (#6230)

* publish release

* Port LiveKernelModel fix to release for point release (#6264)

* Port keybinding updates to release (#6268)

* Contribute shift+enter, ctrl+enter, L, shift+L (#6205)

* Ctrl+Enter should put cell into command mode after executing (#6231)

* Update CHANGELOG and remove news

* port test fix (#6275)

* Fix Restarting kernel... test (#6267)

* wait for the restart command

* add news file

* update changelog

* Fix native notebook interrupt toolbar (#6280) (#6282)

* update changelog (#6284)

* -update version
-update changelog

* leave version as it was

* Respect jupyter.enableKeyboardShortcuts setting and enable ctrl+enter in command mode (#6293)

* publish release

* fix gulpfile

* undo change

Co-authored-by: Ian Huff <ianhu@microsoft.com>
Co-authored-by: Don Jayamanne <don.jayamanne@yahoo.com>
Co-authored-by: Don Jayamanne <don.jayamanne@outlook.com>
Co-authored-by: Joyce Er <joyceerhuiling@gmail.com>
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.

3 participants