Kernel finding refactor #4995
Merged
Kernel finding refactor #4995
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DavidKutu
reviewed
Mar 9, 2021
| @traceDecorators.verbose('Find kernel spec') | ||
| @captureTelemetry(Telemetry.KernelFinderPerf) | ||
| public async findKernel( | ||
| resource: Resource, |
There was a problem hiding this comment.
I'd leave this as is. undefined is supported value.
This way developer needs to make a decision and explicitly pass the value in, instead of accidentally forgetting it.
Fyi, we have had many issues where these (uri) haven't been passed on when they are optional.
DavidKutu
approved these changes
Mar 9, 2021
| } | ||
|
|
||
| // See if the version is the same | ||
| if (interpreter && interpreter.version && spec && spec.name && nbMetadataLanguage === PYTHON_LANGUAGE) { |
There was a problem hiding this comment.
I think this resolves #4941. I was about to take a look at this but figured that I should wait until the refactor. Looks like it should match on a python3 spec name to prefer a python3 interpreter over plain python.
IanMatthewHuff
approved these changes
Mar 9, 2021
DonJayamanne
reviewed
Mar 9, 2021
DonJayamanne
approved these changes
Mar 9, 2021
joyceerhl
reviewed
Mar 9, 2021
joyceerhl
reviewed
Mar 9, 2021
joyceerhl
reviewed
Mar 9, 2021
...st/datascience/jupyter/interpreter/jupyterInterpreterSubCommandExecutionService.unit.test.ts
Show resolved
Hide resolved
joyceerhl
approved these changes
Mar 9, 2021
DonJayamanne
pushed a commit
that referenced
this pull request
Mar 23, 2021
* New idea for kernel finding * Getting some more of the tests running * Move interpreters down to the finder level * Partially building * Change when extensions load * Fix some merge problems * Fix everything except the kernel selector tests * Remove some more code * Remove some more methods * REname build steps * Get everything building * Outline for tests for kernel finders * First local kernel finder test * More tests and handling kernelspecs from interpreters * Fix build errors * List kernels passing * Get all local kernel finder tests to pass * Add remote tests * Rework unit tests for jupyterKernelService * kernel service unit tests passing * Fix sub command unit tests * Fix other tests and eliminate unnecessary test for selections * Fix execution unit tests * Fix display of kernels * Remove dupes that were registered * Turn on linting * Format documents that were missed * Fix build problems * Code review comments * Add telemetry for kernel retry * Add in cancel token and remove unnecessary code * Fix having no interpreter * Fix realpath on Unix * More kernel test fixes * More review feedback and eliminate jupyter tests from functional. They do not run anymore because of usage of file system. * More tests to eliminate * More tests to skip * Put launch.json back * Fix build error * Fix build error * Fix sorting for interpreter based kernels * Use endswith instead of includes * Need user writable location for jupyter kernels * Update comment * Update linter * Put back skip for tests * Notebook metadata not required for a match * Add more logging * More logging and disabling more jupyter tests * Put back getRealPath function. Behavior of nodejs func is different and causes crashes * Skip real path for mac/linux * Fix unit tests and output more information on failures * Make sure true kernel name is in the notebook * Get remote working * Fix linter * Fix problem when no kernel entry in notebook * Fix smoke test and add more logging for vscode tests * More logging and some waiting for a kernel to actually switch * Put open file back * More logging and change kernelspec name to be valid path * Add more logging * Fix id to be unique * Update comment to force another run * Update linter * Use the unique id to compare kernels for equality * Fix remote id problems * More logging and more precise on matches for switch * More logging and try using label * Try more logging * Fix prettier * Add a bunch more logging * Switch doesn't seem to happen before execute * Track kernel data in the document before starting the notebook * More logging * Disable failing tests on jupyter * Fix bug with changing kernel * Skip install when non raw * Figure out correct kernel names * Resolve name during test * Wrong kernel name * Use includes in kernel search too * Try interpreter path instead * Fix tryGetRealPath to actually wait for the result * More tracing and other review feedback * FIx comment on remoteKernelFinder
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rework how we find kernels.
Two new classes:
Both of these classes then use the same algorithm to find matches for a notebook's metadata and/or an interpreter. (Algorithm was moved to a separate file)
Unit tests were made to test the different possible conditions that we have for finding kernels.
All other classes dealing with finding kernels were changed to use these classses. Quick picks for kernels just use these classes and then wrap the result in a quick pick item.