Skip to content

Commit

Permalink
Merge pull request #10185 from microsoft/issue10046Logging
Browse files Browse the repository at this point in the history
  • Loading branch information
DonJayamanne authored May 31, 2022
2 parents 984cfc4 + 968b005 commit fa5c1b4
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 25 deletions.
4 changes: 2 additions & 2 deletions src/kernels/jupyter/launcher/liveshare/hostJupyterServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export class HostJupyterServer implements INotebookServer {
actionSource
);
this.throwIfDisposedOrCancelled(cancelToken);
traceInfo(`Started session for kernel ${kernelConnection.id}`);
traceInfo(`Started session for kernel ${kernelConnection.kind}:${kernelConnection.id}`);
return session;
};

Expand All @@ -121,7 +121,7 @@ export class HostJupyterServer implements INotebookServer {
this.throwIfDisposedOrCancelled(cancelToken);

if (session) {
traceInfo(`Finished connecting kernel ${kernelConnection.id}`);
traceInfo(`Finished connecting kernel ${kernelConnection.kind}:${kernelConnection.id}`);
sessionPromise.resolve(session);
} else {
sessionPromise.reject(this.getDisposedError());
Expand Down
6 changes: 3 additions & 3 deletions src/kernels/kernel.base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -402,9 +402,9 @@ export abstract class BaseKernel implements IKernel {
pythonInfo = ` (${info.join(', ')})`;
}
traceInfo(
`Starting Jupyter Session id = '${this.kernelConnectionMetadata.id}'${pythonInfo} for '${getDisplayPath(
this.id
)}' (disableUI=${this.startupUI.disableUI})`
`Starting Jupyter Session id = '${this.kernelConnectionMetadata.kind}:${
this.kernelConnectionMetadata.id
}'${pythonInfo} for '${getDisplayPath(this.id)}' (disableUI=${this.startupUI.disableUI})`
);
this.createProgressIndicator(disposables);
this.isKernelDead = false;
Expand Down
2 changes: 1 addition & 1 deletion src/notebooks/controllers/notebookControllerManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,7 @@ export class NotebookControllerManager implements INotebookControllerManager, IE
traceVerbose(`Early registration of controller for Kernel connection ${preferredConnection.id}`);
this.createNotebookControllers([preferredConnection]);
}
} else {
} else if (document.notebookType === InteractiveWindowView) {
// Wait for our controllers to be loaded before we try to set a preferred on
// can happen if a document is opened quick and we have not yet loaded our controllers
await loadControllersPromise;
Expand Down
4 changes: 2 additions & 2 deletions src/notebooks/controllers/vscodeNotebookController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -285,8 +285,8 @@ export class VSCodeNotebookController implements Disposable, IVSCodeNotebookCont
private async onDidChangeSelectedNotebooks(event: { notebook: NotebookDocument; selected: boolean }) {
traceInfoIfCI(
`NotebookController selection event called for notebook ${event.notebook.uri.toString()} & controller ${
this.id
}. Selected ${event.selected} `
this.connection.kind
}:${this.id}. Selected ${event.selected} `
);
if (this.associatedDocuments.has(event.notebook) && event.selected) {
// Possible it gets called again in our tests (due to hacks for testing purposes).
Expand Down
45 changes: 29 additions & 16 deletions src/test/datascience/notebook/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -331,30 +331,30 @@ async function waitForKernelToChangeImpl(
const notebookControllers = notebookControllerManager.getRegisteredNotebookControllers();

// Find the kernel id that matches the name we want
let id: string | undefined;
let controller: IVSCodeNotebookController | undefined;
let labelOrId = 'labelOrId' in criteria ? criteria.labelOrId : undefined;
if (labelOrId) {
id = notebookControllers
controller = notebookControllers
?.filter((k) => (criteria.isInteractiveController ? k.id.includes(InteractiveControllerIdSuffix) : true))
?.find((k) => (labelOrId && k.label === labelOrId) || (k.id && k.id == labelOrId))?.id;
if (!id) {
?.find((k) => (labelOrId && k.label === labelOrId) || (k.id && k.id == labelOrId));
if (!controller) {
// Try includes instead
id = notebookControllers?.find(
controller = notebookControllers?.find(
(k) => (labelOrId && k.label.includes(labelOrId)) || (k.id && k.id == labelOrId)
)?.id;
);
}
}
const interpreterPath = 'interpreterPath' in criteria ? criteria.interpreterPath : undefined;
if (interpreterPath && !id) {
id = notebookControllers
if (interpreterPath && !controller) {
controller = notebookControllers
?.filter((k) => k.connection.interpreter)
?.filter((k) => (criteria.isInteractiveController ? k.id.includes(InteractiveControllerIdSuffix) : true))
.find((k) =>
// eslint-disable-next-line local-rules/dont-use-fspath
k.connection.interpreter!.uri.fsPath.toLowerCase().includes(interpreterPath.fsPath.toLowerCase())
)?.id;
);
}
traceInfo(`Switching to kernel id ${id}`);
traceInfo(`Switching to kernel id ${controller}`);
const isRightKernel = () => {
const doc = vscodeNotebook.activeNotebookEditor?.notebook;
if (!doc) {
Expand All @@ -365,7 +365,7 @@ async function waitForKernelToChangeImpl(
if (!selectedController) {
return false;
}
if (selectedController.id === id) {
if (selectedController.id === controller?.id) {
traceInfo(`Found selected kernel id:label ${selectedController.id}:${selectedController.label}`);
return true;
}
Expand All @@ -378,10 +378,17 @@ async function waitForKernelToChangeImpl(
async () => {
// Double check not the right kernel (don't select again if already found to be correct)
if (!isRightKernel() && !skipAutoSelection) {
traceInfoIfCI(`Notebook select.kernel command switching to kernel id ${id}: Try ${tryCount}`);
traceInfoIfCI(
`Notebook select.kernel command switching to kernel id ${controller?.connection.kind}${controller?.id}: Try ${tryCount}`
);
// Send a select kernel on the active notebook editor. Keep sending it if it fails.
await commands.executeCommand('notebook.selectKernel', { id, extension: JVSC_EXTENSION_ID });
traceInfoIfCI(`Notebook select.kernel command switched to kernel id ${id}`);
await commands.executeCommand('notebook.selectKernel', {
id: controller?.id,
extension: JVSC_EXTENSION_ID
});
traceInfoIfCI(
`Notebook select.kernel command switched to kernel id ${controller?.connection.kind}:${controller}`
);
tryCount += 1;
}

Expand Down Expand Up @@ -444,7 +451,9 @@ export async function waitForKernelToGetAutoSelected(
// Do nothing for now. Just log it
traceInfoIfCI(`No preferred controller found during waitForKernelToGetAutoSelected`);
}
traceInfoIfCI(`Wait for kernel - got a preferred notebook controller: ${preferred?.id}`);
traceInfoIfCI(
`Wait for kernel - got a preferred notebook controller: ${preferred?.connection.kind}:${preferred?.id}`
);

// Find one that matches the expected language or the preferred
const expectedLower = expectedLanguage?.toLowerCase();
Expand Down Expand Up @@ -481,7 +490,11 @@ export async function waitForKernelToGetAutoSelected(
}

const criteria = { labelOrId: match!.id };
traceInfo(`Preferred kernel for selection is ${match?.id}, criteria = ${JSON.stringify(criteria)}`);
traceInfo(
`Preferred kernel for selection is ${match.connection.kind}:${match?.id}, criteria = ${JSON.stringify(
criteria
)}`
);
assert.ok(match, 'No kernel to auto select');
return waitForKernelToChange(criteria, timeout, skipAutoSelection);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ suite('DataScience - VSCode Notebook - (Remote) (Execution) (slow)', function ()
await waitForCondition(
async () => controllerManager.getSelectedNotebookController(nbEditor.notebook) === localKernelController,
5_000,
`Controller not swtiched to local kernel, instead it is ${
`Controller not switched to local kernel, instead it is ${
controllerManager.getSelectedNotebookController(nbEditor.notebook)?.id
}`
);
Expand Down

0 comments on commit fa5c1b4

Please sign in to comment.