Fix: dispatch interactive .kql queries to currently selected controller#142
Open
pjanowski wants to merge 1 commit into
Open
Fix: dispatch interactive .kql queries to currently selected controller#142pjanowski wants to merge 1 commit into
pjanowski wants to merge 1 commit into
Conversation
When a query is launched from a .kql file via kusto.executeSelectedQuery, the controller used to execute it is currently the one cached at the time the interactive window was first opened. Changing the kernel via the interactive window's picker updates the UI and globalState but does not affect subsequent runs from the source .kql -- they keep routing through the cached (now stale) controller, AND the per-document Kusto client cache in Client.create also returns a stale client built for the original connection, even if the caller now passes a different connection. Two-part fix: * Track the currently-selected controller per notebook via the existing onDidChangeSelectedNotebooks signal, expose getSelectedController(), and prefer it over the cached controller in executeSelectedQuery. * In Client.create, when the caller passes a connectionInfo that doesn't match the cached client's connectionInfo, drop the stale entry and recreate. Preserves caching when nothing changed.
Author
|
@microsoft-github-policy-service agree company="Microsoft" |
This file contains hidden or 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
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.
Fix: dispatch interactive
.kqlqueries to currently selected controllerFixes #141
Summary
kusto.executeSelectedQuery(run from a.kqlfile via Shift+Enter) currently dispatches to the controller cached at the time the interactive window was first opened. Changing the kernel via the interactive window's picker has no effect on subsequent runs from the source.kql— they continue to execute against the original connection.There are actually two cache layers that need to know about the kernel switch; fixing only one is not enough. This PR addresses both.
What changed
1.
src/extension/kernel/provider.ts— track selected controller per notebook.selectedControllerByNotebook: WeakMap<NotebookDocument, KernelPerConnection>and an exportedgetSelectedController(notebook)lookup.onDidChangeSelectedNotebookshandler inKernelPerConnectionto set/clear the map. Clearing is guarded so an A→B switch where B'sselectfires before A'sdeselectdoesn't accidentally drop B's entry.2.
src/extension/interactive/interactive.ts— use the selected controller at execute time.getSelectedController(notebook)over the cached controller; fall back to the cached one if the selection event hasn't landed yet.3.
src/extension/kusto/client.ts— drop stale client cache when connection changes.Client.createpreviously returned the cachedClientfor the document regardless of theconnectionInfoargument. After (1) + (2),executeCellcorrectly callsClient.create(textDocument, B's connection)— but the cached client (built with A's connection) was still returned.connectionInfothat doesn't match the cached client'sconnectionInfo(lodashisEqual), drop the stale entry and recreate. Caching is preserved when nothing changed.Total: 3 files, ~30 insertions, ~3 deletions. No persistence-format changes, one new public export (
getSelectedController).Repro (before this PR)
.kqlfile. Bind it to cluster A / db A via the$(database)icon..kqlfile, Shift+Enter again on the same block.Before this PR: query still runs against A (the kernel chip says B; execution silently routes to A).
After this PR: query runs against B as expected.
Manual verification
Built as a
.vsix, installed alongside the marketplace 0.5.4 build, validated against a real Kusto cluster (mspva01.westus2.kusto.windows.net / pvananalytics) and a deliberately-wrong cluster (fdislandsus.centralus.kusto.windows.net / CAPAnalytics, which lacks the queried function)..kqlfails with the expected "Unknown function" error against the wrong cluster (no longer silently routing to the original cluster)..knbnotebook path: unchanged — the notebook still binds its kernel via its embedded metadata.Why this is safe
onDidChangeSelectedNotebooksalready fires on the initial controller assignment wheninteractive.openis called with a specific controller id, so the WeakMap is populated by the time the firstexecuteInteractivewould run.?? cachedControllerfallback in interactive.ts preserves current behavior in the pathological case where the selection event has not fired yet.this, which handles both A→B and B→A switch orderings VS Code may produce.Client.createchange only changes behavior when the caller explicitly passes aconnectionInfothat differs from the cached client's — when noconnectionInfois passed (theensureDocumentHasConnectionInfofallback path), behavior is unchanged.Notes for reviewers
connectionreference (nothis.) in the last line of theonDidChangeSelectedNotebookshandler was left as-is to minimize the diff.isEqualto handle the discriminatedIConnectionInfounion (azAuth vs appInsights) without special-casing fields. lodash is already a transitive dep..kqlper-file connection via the editor title-bar$(database)icon (not the kernel picker) — also doesn't fully take effect on an already-open interactive window. The cached controller ininteractive.ts'sdocumentInteractiveDocumentsis unchanged byonDidChangeConnection. This PR does not address that; it's a follow-up.npm run compiledoesn't include the server bundle (src/server/);npm run compile:webpackdoes. Mentioning in case a CI script needs updating.