Skip to content

Commit

Permalink
Fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
DonJayamanne committed Jun 28, 2022
1 parent 4050478 commit 579f1f1
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 12 deletions.
2 changes: 1 addition & 1 deletion src/kernels/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1534,7 +1534,7 @@ export async function executeSilently(
handleExecuteSilentErrors(outputs, errorOptions, codeForLogging);
}

traceInfo(`Executing silently Code (completed) = ${codeForLogging}`);
traceInfo(`Executing silently Code (completed) = ${codeForLogging} with ${outputs.length} output(s)`);

return outputs;
}
Expand Down
25 changes: 22 additions & 3 deletions src/kernels/ipywidgets/baseIPyWidgetScriptManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@ import { Uri } from 'vscode';
import { disposeAllDisposables } from '../../platform/common/helpers';
import { getDisplayPath } from '../../platform/common/platform/fs-paths';
import { IDisposable } from '../../platform/common/types';
import { traceWarning } from '../../platform/logging';
import { traceError, traceInfoIfCI, traceWarning } from '../../platform/logging';
import { sendTelemetryEvent, Telemetry } from '../../telemetry';
import { IKernel, isLocalConnection } from '../types';
import { getTelemetrySafeHashedString } from '../../platform/telemetry/helpers';
import * as stripComments from 'strip-comments';
import { IIPyWidgetScriptManager } from './types';
import { StopWatch } from '../../platform/common/utils/stopWatch';
import { isCI } from '../../platform/common/constants';

export function extractRequireConfigFromWidgetEntry(baseUrl: Uri, widgetFolderName: string, contents: string) {
// Look for `require.config(` or `window["require"].config` or `window['requirejs'].config`
Expand Down Expand Up @@ -148,9 +149,21 @@ export abstract class BaseIPyWidgetScriptManager implements IIPyWidgetScriptMana

try {
const config = await extractRequireConfigFromWidgetEntry(baseUrl, widgetFolderName, contents);
if (!config) {
let message = `Failed to extract require.config from widget for ${widgetFolderName} from ${getDisplayPath(
script
)}`;
if (isCI) {
message += `with contents ${contents}`;
}
traceWarning(message);
}
return config;
} catch (ex) {
traceWarning(`Failed to extract require.config entry from ${getDisplayPath(script)}`, ex);
traceError(
`Failed to extract require.config entry for ${widgetFolderName} from ${getDisplayPath(script)}`,
ex
);
}
}
private async getWidgetModuleMappingsImpl(): Promise<Record<string, Uri> | undefined> {
Expand All @@ -168,11 +181,17 @@ export abstract class BaseIPyWidgetScriptManager implements IIPyWidgetScriptMana

const config = widgetConfigs.reduce((prev, curr) => Object.assign(prev || {}, curr), {});
// Exclude entries that are not required (widgets that we have already bundled with our code).
if (config) {
if (config && Object.keys(config).length) {
delete config['jupyter-js-widgets'];
delete config['@jupyter-widgets/base'];
delete config['@jupyter-widgets/controls'];
delete config['@jupyter-widgets/output'];
} else {
traceInfoIfCI(
`No config, entryPoints = ${JSON.stringify(entryPoints)}, widgetConfigs = ${JSON.stringify(
widgetConfigs
)}`
);
}
sendTelemetryEvent(Telemetry.DiscoverIPyWidgetNamesPerf, stopWatch.elapsedTime, {
type: isLocalConnection(this.kernel.kernelConnectionMetadata) ? 'local' : 'remote'
Expand Down
23 changes: 16 additions & 7 deletions src/kernels/ipywidgets/remoteIPyWidgetScriptManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import * as path from '../../platform/vscode-path/path';
import * as dedent from 'dedent';
import { ExtensionMode, Uri } from 'vscode';
import { IExtensionContext, IHttpClient } from '../../platform/common/types';
import { traceError } from '../../platform/logging';
import { traceError, traceInfoIfCI } from '../../platform/logging';
import { executeSilently, isPythonKernelConnection } from '../helpers';
import { IKernel, RemoteKernelConnectionMetadata } from '../types';
import { IIPyWidgetScriptManager } from './types';
Expand Down Expand Up @@ -57,13 +57,14 @@ export class RemoteIPyWidgetScriptManager extends BaseIPyWidgetScriptManager imp
}

const code = dedent`
__vsc_nbextension_widgets = []
__vsc_file = ''
__vsc_nbextension_Folder = ''
import glob as _VSCODE_glob
import os as _VSCODE_os
import sys as _VSCODE_sys
try:
__vsc_nbextension_widgets = []
import glob as _VSCODE_glob
import os as _VSCODE_os
import os as _VSCODE_sys
__vsc_nbextension_Folder = _VSCODE_sys.prefix + _VSCODE_os.path.sep + 'share' + _VSCODE_os.path.sep + 'jupyter' + _VSCODE_os.path.sep + 'nbextensions' + _VSCODE_os.path.sep
__vsc_file = ''
for __vsc_file in _VSCODE_glob.glob(__vsc_nbextension_Folder + '*' + _VSCODE_os.path.sep + 'extension.js'):
__vsc_nbextension_widgets.append(__vsc_file.replace(__vsc_nbextension_Folder, ""))
Expand All @@ -79,10 +80,16 @@ export class RemoteIPyWidgetScriptManager extends BaseIPyWidgetScriptManager imp
del __vsc_nbextension_Folder
del __vsc_nbextension_widgets`;
if (!this.kernel.session) {
traceInfoIfCI('No Kernel session to get list of widget entry points');
return [];
}
const promises: Promise<nbformat.IOutput[]>[] = [];
promises.push(executeSilently(this.kernel.session, code));
promises.push(
executeSilently(this.kernel.session, code, {
traceErrors: true,
traceErrorsMessage: 'Failed to get widget entry points from remote kernel'
})
);
// A bug was identified in our code that resulted in a deadlock.
// While the widgets are loading this code gets executed, however the kernel execution is blocked waiting for kernel messages to be completed on the UI side
// This is how we synchronize messages between the UI and kernel - i.e. we wait for kernel messages to be handled completely.
Expand All @@ -101,10 +108,12 @@ export class RemoteIPyWidgetScriptManager extends BaseIPyWidgetScriptManager imp

const outputs = await Promise.race(promises);
if (outputs.length === 0) {
traceInfoIfCI('Unable to get widget entry points, no outputs after running the code');
return [];
}
const output = outputs[0] as nbformat.IStream;
if (output.output_type !== 'stream' || output.name !== 'stdout') {
traceInfoIfCI('Unable to get widget entry points, no stream/stdout outputs after running the code');
return [];
}
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ suite(`Interactive window execution`, async function () {
}
sinon.restore();
await closeNotebooksAndCleanUpAfterTests(disposables);
traceInfo(`Ended Test (completed) ${this.currentTest?.title}`);
});

test('Execute cell from Python file', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ suite('IPyWidget Script Manager', function () {
kernel = kernelProvider.get(notebook.uri)!;
scriptManager = widgetScriptManagerFactory.getOrCreate(kernel);
});
setup(async function () {
traceInfo(`Starting Test ${this.currentTest?.title}`);
});

teardown(async function () {
traceInfo(`Ended Test ${this.currentTest?.title}`);
if (this.currentTest?.isFailed()) {
Expand Down Expand Up @@ -175,6 +179,7 @@ suite('IPyWidget Script Manager', function () {
});
test('Should not contain any modules that we already bundle with our ipywidgets bundle', async () => {
const moduleMappings = await scriptManager.getWidgetModuleMappings();
assert.isObject(moduleMappings);
['jupyter-js-widgets', '@jupyter-widgets/base', '@jupyter-widgets/controls', '@jupyter-widgets/output'].forEach(
(moduleName) => {
assert.isFalse(moduleName in moduleMappings!, `Module ${moduleName} should not exist in the mapping`);
Expand Down
1 change: 0 additions & 1 deletion src/test/datascience/widgets/rendererUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ function initializeComms() {
return;
}
rendererContext.onDidReceiveMessage((message) => {
console.log(`Received message in Widget renderer ${JSON.stringify(message)}`);
rendererContext.postMessage!({
command: 'log',
message: `Received message in Widget renderer ${JSON.stringify(message)}`
Expand Down

0 comments on commit 579f1f1

Please sign in to comment.