if .ipynb document metadata isn't present, fall back to interpreting the kernelspec #2692
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
brettfo
commented
Feb 7, 2023
| const cellMetadata = metadataUtilities.getNotebookCellMetadataFromNotebookCellElement(cell); | ||
| const cellKernelName = cellMetadata.kernelName ?? 'csharp'; | ||
| const cellKernelName = cellMetadata.kernelName ?? notebookMetadata.kernelInfo.defaultKernelName; |
There was a problem hiding this comment.
This is the core of the fix; everything else is just making sure we have something meaningful here.
brettfo
commented
Feb 7, 2023
| const previousCellMetadata = metadataUtilities.getNotebookCellMetadataFromNotebookCellElement(previousCell); | ||
| await vscodeUtilities.setCellKernelName(foundCell, previousCellMetadata.kernelName ?? notebookMetadata.kernelInfo.defaultKernelName); | ||
| // no kernel name; set it from the notebook metadata | ||
| await vscodeUtilities.setCellKernelName(foundCell, notebookMetadata.kernelInfo.defaultKernelName); |
There was a problem hiding this comment.
This is part 2 of the core fix; when we first open a document, we ensure each cell has a kernel name set.
jonsequitur
approved these changes
Feb 7, 2023
colombod
approved these changes
Feb 7, 2023
colombod
added a commit
that referenced
this pull request
Feb 9, 2023
* rename --from-value to --value * use formatted value in the value kernel * api contract change * test fallback to text/plain test * test value info produced * change default HTML formatting to a tree layout rather than a table (#2671) * split HTML formatter tests into smaller files * split PlainTextFormatterTests into smaller files * refactoring * implement tree views for objects * remove HtmlFormatter.MaxProperties * improve dictionary and sequence rendering, open top level of treeview * remove old object table formatters, clean up * Links for mermaid and kql added As a convenience i added links for kql and mermaid because i assume that these are lesser well known * improve variable explorer view (#2634) * implementation for filter and resize restore update fonts remove dummy data * rename * disable build (#2687) * delete my binder * Value explorer improvements (#2690) * change header text * fix input * use routing slip and intercept completions of sendvalue * Enforce a minimum version for Kusto and SQL Service dotnet tools. (#2689) * Enforce a minimum version for Kusto and SQL Service dotnet tools. * Update name. * Specify nuget package name. * Refactor install code and add update * Add quotes around provided arguments * Make classes internal * Remove internal Utils class from API compatibility tests. * Update formatting.md * replace Clear() by ResetToDefault() * replace 404 urls for default formatters * if .ipynb document metadata isn't present, fall back to interpreting the kernelspec (#2692) * skip ci on certain changes (#2695) * rename folders (#2696) * wip * wip on changing KernelCommandResult.KernelEvents from IObservable to list * more API usage cleanup * remove KernelCommandResult.KernelEvents * cleanup * enable ellispsis (#2701) * enable ellispsis adding tooltips * use percentage test track affected column on drag start * merge changes with main and make fixes * fix (#2706) --------- Co-authored-by: Diego Colombo <[email protected]> Co-authored-by: Jon Sequeira <[email protected]> Co-authored-by: surfmuggle <[email protected]> Co-authored-by: Diego Colombo <[email protected]> Co-authored-by: Cory Rivera <[email protected]> Co-authored-by: Marc Kruzik <[email protected]> Co-authored-by: Brett V. Forsgren <[email protected]>
jonsequitur
added a commit
that referenced
this pull request
Apr 6, 2023
* update tool feeds (#2258) * Update all usages of NetCore* Pool providers to -Svc version since this is a "release/" branch Eng/common changes will be the same in next arcade update from the release/7.0 branch if/when this is taken. * add kernel name to Jupyter metadata (#2362) * align TabularDataResource formatting for json to table-schema+json * update System.Security.Cryptography.Xml package * introduce jupyter kernel and handle communication over the http * move to use Proxy kernel * add more message observable extensions * handle protocol serialization exceptions * basic variable sharing support added. * refactor to separate jupyter connection from jupyter kernel connection and add kernel info to the proxy kernel. * make command handling using dynamic command handler registers to allow language specific handling. * add completion for juptyer kernels * add basic support for hover text. NOTE: currently the text is rendered with the ansi encoding. * add support to set dataframe in python and refactor to initialize a value handler on kernel info retrurned. R value support needs more work. * fix set data frame in R * intialize value handling properly as the kernel is always expected to handle get value and valueinfo now * enable connections that require a bearer token over token auth * python comm based value adapter * enable R for data sharing via comm adapter * add get request support * add support for get variables to python * add get variable for R * add request value info for all kernels * initial zmq connection logic * fix rebase issues. * normalize line endings and change namings * update with new code changes. * properly tag the new files * fix R to python dataframe marshalling * remove value declarer and use sendvalue command instead. * fix to use the TabularDataResourceFormatter instead of using default serialization as the internal types can change and have changed. Python and R apply the appropriate changes to handle this. * cleanup some of the message handling and fix kernel ids and URIs * refactor names to make them more understandable * fix normalized line endings to not be public contract * add completions for list kernel specs. Cannot parse shared variable tokens * fix so that we do null check for kernel specs. * set up ZMQ connections to look up directories correctly * pr feedback on names and type updates * Detect and report on invalid variable identifiers. Also inform of variable names when storing multiple result sets. * enable update_display_data * rename exception to start * strip away unsupported formats coming from the kernel on documentation * refactor jupyter http connection * Refactor from Proxy kernel to inherit from kernel * fix the api client names * fix submit code to wait on executereply as well * refactor out unnecessary code * change run on kernel to async method * handle kernel interrupts on cancel * move process to kernel connection * add other channels to send to in zmq * use kernelspec list to locate local kernels. There is a perf impact to this as the kernel specs are all returned. * add comm channel lookup. Pending: differentiate between local comms vs comms on kernel * change local connection to a singleton and pre-retrieve all the available specs to help with performance. * enable stdin * log zmq process stdio * add images to root for the marketplace links * use absolute url for images * js kernel can evaluate javascript code * use async evaluator * add images to root for the marketplace links removing interactive window from vscode insiders Revert "removing interactive window from vscode insiders" This reverts commit 243eacf. deprecate command remove commands and activations remove controllers and commands set it back put back fix white spaces add tag add tag * fixing case for variable explorer * remove left over test code * add kusto notebook * kql and sql fix * ".NET Interactive" "dotnet interactive" * decouple sub kernel handling command events from comms manager. * add command to run script on initialization * create a mew command event adapter logic for value sharing * rename adapter to channel and add support for request value and request value info * re-enable all value sharing commands using commandevents comm channel for R * delete all the DAP related protocol messages * add licensing * fix API tests for jupyter with new changes * fix stale naming * update the scripts for python and r to handle specific message type * fix formatting from merge issues * move extension loading to tool project, improve extension.dib loading (#2656) * move extension loading to tool project, improve extension.dib loading * put back LogEventsToPocketLogger as internal * update HTTP API test to use UseNuGetExtensions * wip --from-result * wip * more tests * setting value from interpolation * get value from another kernel * wip * refactor set magic command as part of the value sharing gesture * specifying mimetype ignores reference value * Add the test infra layer for running tests against Jupyter connection - Part 1 (#2703) * make jupyter connect options extendible via a new interface * test framework to start recording and running tests on both server and zmq * add more tests and add serialization tests * clean up tests * add licensing * make tests more specific * make tests skippable based on env variable * change the testconnection contracts to use test specific types * fix deserialization bug where empty json could not converted to the given msg content type which could be empty * enable the tests to be recorded and played back when there is no setup or connection * add a developer guide * add the test records * remove internal logistics from tests and add assertions on the result instead * fix build error * merge `main` into `release/jupyter` (#2705) * rename --from-value to --value * use formatted value in the value kernel * api contract change * test fallback to text/plain test * test value info produced * change default HTML formatting to a tree layout rather than a table (#2671) * split HTML formatter tests into smaller files * split PlainTextFormatterTests into smaller files * refactoring * implement tree views for objects * remove HtmlFormatter.MaxProperties * improve dictionary and sequence rendering, open top level of treeview * remove old object table formatters, clean up * Links for mermaid and kql added As a convenience i added links for kql and mermaid because i assume that these are lesser well known * improve variable explorer view (#2634) * implementation for filter and resize restore update fonts remove dummy data * rename * disable build (#2687) * delete my binder * Value explorer improvements (#2690) * change header text * fix input * use routing slip and intercept completions of sendvalue * Enforce a minimum version for Kusto and SQL Service dotnet tools. (#2689) * Enforce a minimum version for Kusto and SQL Service dotnet tools. * Update name. * Specify nuget package name. * Refactor install code and add update * Add quotes around provided arguments * Make classes internal * Remove internal Utils class from API compatibility tests. * Update formatting.md * replace Clear() by ResetToDefault() * replace 404 urls for default formatters * if .ipynb document metadata isn't present, fall back to interpreting the kernelspec (#2692) * skip ci on certain changes (#2695) * rename folders (#2696) * wip * wip on changing KernelCommandResult.KernelEvents from IObservable to list * more API usage cleanup * remove KernelCommandResult.KernelEvents * cleanup * enable ellispsis (#2701) * enable ellispsis adding tooltips * use percentage test track affected column on drag start * merge changes with main and make fixes * fix (#2706) --------- Co-authored-by: Diego Colombo <[email protected]> Co-authored-by: Jon Sequeira <[email protected]> Co-authored-by: surfmuggle <[email protected]> Co-authored-by: Diego Colombo <[email protected]> Co-authored-by: Cory Rivera <[email protected]> Co-authored-by: Marc Kruzik <[email protected]> Co-authored-by: Brett V. Forsgren <[email protected]> * add tests for submit code and it's results (#2708) * add tests for streaming stdout and stderr and also change to validate all the formatted outputs in display value * add error produced tests * add test for python update display value produced * fix formatting * add more tests for jupyter kernel directly * add more tests for validating kernel behavior * add hover text test * add kernel language tests (#2721) * change log level (#2719) * add error message returned scenarios * add sig help and completion error scenario * add cancelation tests for commands * added signature help tests for python and disposed check * clean up signature help tests * add test for completion requests * add a check for the received messages as we well for completion list * fix indentation in ts contracts --------- Co-authored-by: Diego Colombo <[email protected]> * Jupyter/fix completion kind (#2730) * change log level (#2719) * use the completion metadata for for experimental types to get additional information on the completions --------- Co-authored-by: Diego Colombo <[email protected]> * [release/jupyter] fix tool source (#2748) * fix tool source * fix build pool * unit tests for the python command handler * add error scenarios for commands * add tests for request all and bat file to run the tests * add test for invalid json * r tests * add more r tests and fix a bug in r handler * test dataframe set and get in python * fix a bug in r that caused data frame to fail * cleanup and add more r unit tests * merge from main (#2754) * change log level (#2719) * fix handling of completions that rely on nested commands (#2727) * fix handling of completions that rely on nested commands * add logic to compensate for #2728 * fix package import (#2734) * Rename the netstandard2.0 package (#2739) Also include some additional code that we want to share. Hide command window for child process. * value kernel should not produce reference values (#2740) fix fix * readme link 404 fix corrected path for FAQ.md * share cancellation source (#2743) error * fix size in style * use set command * Update publish-npm.yml * report all errors when generating dynamic semantic tokens (#2747) * report all errors when generating dynamic semantic tokens * report when no tokens were produced * fix again ... (#2749) * improve kql and sql connection experience (#2745) * add tests to see reconnection dive out error if kernel name is already present add tests fix test * fix test * Update src/Microsoft.DotNet.Interactive.Kql/ConnectKqlCommand.cs Co-authored-by: Jon Sequeira <[email protected]> * Update src/Microsoft.DotNet.Interactive.SqlServer.Tests/MsSqlConnectionTests.cs Co-authored-by: Jon Sequeira <[email protected]> * fix test --------- Co-authored-by: Jon Sequeira <[email protected]> * Update README.md * Updating the test script with the latest saved content for .dib and .ipynb files and fixing some typos * allow persisting default values for new notebooks (#2712) --------- Co-authored-by: Diego Colombo <[email protected]> Co-authored-by: Jon Sequeira <[email protected]> Co-authored-by: Shyam N <[email protected]> Co-authored-by: Jamie Bilinski <[email protected]> Co-authored-by: Diego Colombo <[email protected]> Co-authored-by: Brett V. Forsgren <[email protected]> Co-authored-by: Abhitej John Bandi <[email protected]> * Jupyter/developer guide updates (#2761) * fix the developer guide * add licensing info for any inspirational projects * Jupyter/variable sharing e2e tests pt1 (#2764) * enable variable sharing for the composite kernel in jupyter kernel tests * add variable e2e tests for primitives roundtrip sharing and dataframe round trip sharing * add other connection types * update license info * add tests for different use cases * add request value info tests and error cases for requestvalue * fix header * fix the random chars * go back to the original wording * shutdown active sessions when kernel is disposed. * fix bug where interrupt was called on a disposed object * adopt the contract change for requestvalueinfos * update the test messages * make sure that kernelspecs can be returned in the completions * Jupyter/merge from main (#2774) * change log level (#2719) * fix handling of completions that rely on nested commands (#2727) * fix handling of completions that rely on nested commands * add logic to compensate for #2728 * fix package import (#2734) * Rename the netstandard2.0 package (#2739) Also include some additional code that we want to share. Hide command window for child process. * value kernel should not produce reference values (#2740) fix fix * readme link 404 fix corrected path for FAQ.md * share cancellation source (#2743) error * fix size in style * use set command * Update publish-npm.yml * report all errors when generating dynamic semantic tokens (#2747) * report all errors when generating dynamic semantic tokens * report when no tokens were produced * fix again ... (#2749) * improve kql and sql connection experience (#2745) * add tests to see reconnection dive out error if kernel name is already present add tests fix test * fix test * Update src/Microsoft.DotNet.Interactive.Kql/ConnectKqlCommand.cs Co-authored-by: Jon Sequeira <[email protected]> * Update src/Microsoft.DotNet.Interactive.SqlServer.Tests/MsSqlConnectionTests.cs Co-authored-by: Jon Sequeira <[email protected]> * fix test --------- Co-authored-by: Jon Sequeira <[email protected]> * Update README.md * Updating the test script with the latest saved content for .dib and .ipynb files and fixing some typos * allow persisting default values for new notebooks (#2712) * cancelling doesn't leak infos * cleanup * throw on null FormattedValue * move #!set handling to custom parser, add support for --byref * cleanup * move #!set --value @sharing to custom parser, add --byref * fix sql test assertion * honour content type * Update dependencies from https://github.com/dotnet/arcade build 20230222.4 (#2772) [main] Update dependencies from dotnet/arcade * Remove not needed code * upgraded js --------- Co-authored-by: Diego Colombo <[email protected]> Co-authored-by: Jon Sequeira <[email protected]> Co-authored-by: Shyam N <[email protected]> Co-authored-by: Jamie Bilinski <[email protected]> Co-authored-by: Diego Colombo <[email protected]> Co-authored-by: Brett V. Forsgren <[email protected]> Co-authored-by: Abhitej John Bandi <[email protected]> Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Andrii Kurdiumov <[email protected]> * revert tool source changes to not use experiemental package source * cleanup unneeded changes from merges and address some PR feedback * rename to appropriately represent environment * use shorter name * adapt to send back an empty kernelinfo list for kernel ready. We only use this for kernel variable sharing and not for kernel initialization. * update the test records to include the fix for the contract change. Since the change applies to kernel variable sharing initialization, it impacts all tests. * PR feedback on the developer guide * use are more specific kernel uri * Give the jupyter kernel a display name with "Preview" tag in it. * move the logic for the current connection from JupyterConnection to the Command Options * fix data frame sharing tests * add checks for messages being sent to the kernel for value sharing * add preview languages via jupyter added to extension readmes * update to clarify better in readme * fix R kernel tests to enable them to run from command line. * fix the experience for newly installed conda where there is no runtime directory under jupyter folder until jupyter actually runs. * Don't set URI when creating a kernel since we are not a proxy kernel anymore. --------- Co-authored-by: Brett V. Forsgren <[email protected]> Co-authored-by: Matt Galbraith <[email protected]> Co-authored-by: shibbas <[email protected]> Co-authored-by: Jon Sequeira <[email protected]> Co-authored-by: Diego Colombo <[email protected]> Co-authored-by: Diego Colombo <[email protected]> Co-authored-by: surfmuggle <[email protected]> Co-authored-by: Cory Rivera <[email protected]> Co-authored-by: Marc Kruzik <[email protected]> Co-authored-by: Shyam N <[email protected]> Co-authored-by: Jamie Bilinski <[email protected]> Co-authored-by: Abhitej John Bandi <[email protected]> Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Andrii Kurdiumov <[email protected]>
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.
If a cell doesn't have kernel metadata, fall back to using the document's metadata, and if the document doesn't have the appropriate metadata, re-create it from the kernelspec.
This bug was only in the VS Code extension; the document APIs already has a test for exactly this scenario here.
Fixes #2685.