Skip to content

Load widget scripts from CDN and/or local python interpreter #10987

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Don Jayamanne (DonJayamanne) merged 50 commits into from
Apr 10, 2020
Merged

Load widget scripts from CDN and/or local python interpreter #10987

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
4348575
Disable loading scripts from unpkg for widgets
DonJayamanne Apr 2, 2020
cca2194
Oops
DonJayamanne Apr 2, 2020
f9cf82f
Updated
DonJayamanne Apr 2, 2020
dc69573
review fixes
DonJayamanne Apr 2, 2020
21e3191
Backup
DonJayamanne Apr 3, 2020
ebf5ea6
Merge branch 'master' into local2
DonJayamanne Apr 6, 2020
b399d2e
Merge branch 'master' into local2
DonJayamanne Apr 6, 2020
1675a7b
Merge branch 'master' into local2
DonJayamanne Apr 6, 2020
1ac4458
Ooops
DonJayamanne Apr 6, 2020
91f9d2d
Misc
DonJayamanne Apr 7, 2020
1dbf99d
Oops
DonJayamanne Apr 7, 2020
278341a
Merge branch 'master' into local2
DonJayamanne Apr 7, 2020
78564d6
Fixes
DonJayamanne Apr 7, 2020
544e740
misc
DonJayamanne Apr 8, 2020
19751e9
misc
DonJayamanne Apr 8, 2020
3b6861c
Merge branch 'master' into local2
DonJayamanne Apr 8, 2020
1df1e10
Remove
DonJayamanne Apr 8, 2020
11499bb
Fixes
DonJayamanne Apr 8, 2020
953b5c4
Changes
DonJayamanne Apr 8, 2020
4f71587
Merge branch 'master' into local2
DonJayamanne Apr 8, 2020
0f9a413
Fix casing
DonJayamanne Apr 9, 2020
2342dfc
Misc changes
DonJayamanne Apr 9, 2020
4afd7bf
Fixes
DonJayamanne Apr 9, 2020
1cf388e
Merge branch 'local3' into local2
DonJayamanne Apr 9, 2020
2ee0597
Merge branch 'master' into local2
DonJayamanne Apr 9, 2020
ddd1d9d
Oops
DonJayamanne Apr 9, 2020
f785d94
Remove azure ML
DonJayamanne Apr 9, 2020
aaf7407
Oops
DonJayamanne Apr 9, 2020
b0d436f
Rename test
DonJayamanne Apr 9, 2020
7a5f73c
Review fixes
DonJayamanne Apr 9, 2020
c4deb45
More fixes
DonJayamanne Apr 9, 2020
8e5ad7a
More changes
DonJayamanne Apr 9, 2020
3aba931
More changes
DonJayamanne Apr 9, 2020
f1a4c1b
Faster check
DonJayamanne Apr 9, 2020
798e8ee
More changes after discussion
DonJayamanne Apr 9, 2020
bfb7792
Fix tests
DonJayamanne Apr 9, 2020
dd87682
Fix tests
DonJayamanne Apr 9, 2020
ded7fbf
Fix
DonJayamanne Apr 9, 2020
075db59
Restore AML
DonJayamanne Apr 9, 2020
f8061fc
Final changes
DonJayamanne Apr 9, 2020
4a95e6c
Merge branch 'master' into local2
DonJayamanne Apr 9, 2020
3b91f08
Merged
DonJayamanne Apr 9, 2020
51b9803
Oops
DonJayamanne Apr 9, 2020
f948a09
OOps
DonJayamanne Apr 9, 2020
ce92590
Address sonar issues
DonJayamanne Apr 9, 2020
5f370c9
Fix linter
DonJayamanne Apr 9, 2020
92e056e
Fixes
DonJayamanne Apr 10, 2020
ede21dc
Better way to pas array buffer
DonJayamanne Apr 10, 2020
d9f0d16
Added comments
DonJayamanne Apr 10, 2020
b273642
Oops
DonJayamanne Apr 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix tests
  • Loading branch information
@DonJayamanne
DonJayamanne committed Apr 9, 2020
commit bfb7792d20414ef44134096d99148f6ceed69c54
16 changes: 2 additions & 14 deletions src/client/datascience/ipywidgets/ipyWidgetScriptSourceProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,14 +141,14 @@ export class IPyWidgetScriptSourceProvider implements IWidgetScriptSourceProvide
private rebuildProviders() {
this.disposeScriptProviders();
// If we haven't configured anything, then nothing to do here.
if (this.configuredScriptSources.length === 0) {
if (this.configuredScriptSources.length === 0 && !this.userConfiguredCDNAtLeastOnce.value) {
return;
}

const scriptProviders: IWidgetScriptSourceProvider[] = [];

// If we're allowed to use CDN providers, then use them, and use in order of preference.
if (this.canUseCDN()) {
if (this.configuredScriptSources.length > 0) {
scriptProviders.push(new CDNWidgetScriptSourceProvider(this.configurationSettings, this.httpClient));
}
if (this.notebook.connection.localLaunch) {
Expand All @@ -166,18 +166,6 @@ export class IPyWidgetScriptSourceProvider implements IWidgetScriptSourceProvide

this.scriptProviders = scriptProviders;
}
private canUseCDN(): boolean {
if (!this.notebook) {
return false;
}
const settings = this.configurationSettings.getSettings(undefined);
const scriptSources = settings.datascience.widgetScriptSources;
if (scriptSources.length === 0) {
return false;
}

return scriptSources.indexOf('jsdelivr.com') >= 0 || scriptSources.indexOf('unpkg.com') >= 0;
}

private async configureWidgets(): Promise<void> {
if (this.configuredScriptSources.length !== 0) {
Expand Down
179 changes: 71 additions & 108 deletions src/test/datascience/ipywidgets/ipyWidgetScriptSourceProvider.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ suite('Data Science - ipywidget - Widget Script Source Provider', () => {
let appShell: IApplicationShell;
let workspaceService: IWorkspaceService;
let onDidChangeWorkspaceSettings: EventEmitter<ConfigurationChangeEvent>;
let state: PersistentState<boolean>;
let userSelectedOkOrDoNotShowAgainInPrompt: PersistentState<boolean>;
setup(() => {
notebook = mock(JupyterNotebookBase);
configService = mock(ConfigurationService);
Expand All @@ -45,11 +45,15 @@ suite('Data Science - ipywidget - Widget Script Source Provider', () => {
const fs = mock(FileSystem);
const interpreterService = mock(InterpreterService);
const stateFactory = mock(PersistentStateFactory);
state = mock<PersistentState<boolean>>();
userSelectedOkOrDoNotShowAgainInPrompt = mock<PersistentState<boolean>>();

when(stateFactory.createGlobalPersistentState(anything(), anything())).thenReturn(instance(state));
when(stateFactory.createGlobalPersistentState(anything(), anything())).thenReturn(
instance(userSelectedOkOrDoNotShowAgainInPrompt)
);
settings = { datascience: { widgetScriptSources: [] } } as any;
when(configService.getSettings(anything())).thenReturn(settings as any);
when(userSelectedOkOrDoNotShowAgainInPrompt.value).thenReturn(false);
when(userSelectedOkOrDoNotShowAgainInPrompt.updateValue(anything())).thenResolve();
CDNWidgetScriptSourceProvider.validUrls = new Map<string, boolean>();
scriptSourceProvider = new IPyWidgetScriptSourceProvider(
instance(notebook),
Expand Down Expand Up @@ -84,79 +88,105 @@ suite('Data Science - ipywidget - Widget Script Source Provider', () => {
assert.deepEqual(values, []);
});
test('Prompt to use CDN', async () => {
when(appShell.showInformationMessage(anything(), anything(), anything())).thenResolve();
when(appShell.showInformationMessage(anything(), anything(), anything(), anything())).thenResolve();

await scriptSourceProvider.getWidgetScriptSource('HelloWorld', '1');

verify(
appShell.showInformationMessage(DataScience.useCDNForWidgets(), Common.ok(), Common.cancel())
appShell.showInformationMessage(
DataScience.useCDNForWidgets(),
Common.ok(),
Common.cancel(),
Common.doNotShowAgain()
)
).once();
});
test('Do not prompt to use CDN if user has chosen not to use a CDN', async () => {
when(appShell.showInformationMessage(anything(), anything(), anything(), anything())).thenResolve();
when(userSelectedOkOrDoNotShowAgainInPrompt.value).thenReturn(true);

await scriptSourceProvider.getWidgetScriptSource('HelloWorld', '1');

verify(
appShell.showInformationMessage(
DataScience.useCDNForWidgets(),
Common.ok(),
Common.cancel(),
Common.doNotShowAgain()
)
).never();
});
function verifyNoCDNUpdatedInSettings() {
// Confirm message was displayed.
verify(
appShell.showInformationMessage(DataScience.useCDNForWidgets(), Common.ok(), Common.cancel())
appShell.showInformationMessage(
DataScience.useCDNForWidgets(),
Common.ok(),
Common.cancel(),
Common.doNotShowAgain()
)
).once();

// Confirm settings were updated.
verify(
configService.updateSetting(
'dataScience.widgets.localConnectionScriptSources',
deepEqual(['localPythonEnvironment']),
undefined,
ConfigurationTarget.Global
)
).once();
verify(
configService.updateSetting(
'dataScience.widgets.remoteConnectionScriptSources',
deepEqual(['remoteJupyterServer']),
'dataScience.widgetScriptSources',
deepEqual([]),
undefined,
ConfigurationTarget.Global
)
).once();
}
test('Update settings to not use CDN if prompt is dismissed', async () => {
when(appShell.showInformationMessage(anything(), anything(), anything())).thenResolve();
test('Do not update if prompt is dismissed', async () => {
when(appShell.showInformationMessage(anything(), anything(), anything(), anything())).thenResolve();

await scriptSourceProvider.getWidgetScriptSource('HelloWorld', '1');

verifyNoCDNUpdatedInSettings();
verify(configService.updateSetting(anything(), anything(), anything(), anything())).never();
verify(userSelectedOkOrDoNotShowAgainInPrompt.updateValue(true)).never();
});
test('Update settings to not use CDN if Cancel is clicked in prompt', async () => {
when(appShell.showInformationMessage(anything(), anything(), anything())).thenResolve(
test('Do not update settings if Cancel is clicked in prompt', async () => {
when(appShell.showInformationMessage(anything(), anything(), anything(), anything())).thenResolve(
Common.cancel() as any
);

await scriptSourceProvider.getWidgetScriptSource('HelloWorld', '1');

verify(configService.updateSetting(anything(), anything(), anything(), anything())).never();
verify(userSelectedOkOrDoNotShowAgainInPrompt.updateValue(true)).never();
});
test('Update settings to not use CDN if `Do Not Show Again` is clicked in prompt', async () => {
when(appShell.showInformationMessage(anything(), anything(), anything(), anything())).thenResolve(
Common.doNotShowAgain() as any
);

await scriptSourceProvider.getWidgetScriptSource('HelloWorld', '1');

verifyNoCDNUpdatedInSettings();
verify(userSelectedOkOrDoNotShowAgainInPrompt.updateValue(true)).once();
});
test('Update settings to use CDN based on prompt', async () => {
when(appShell.showInformationMessage(anything(), anything(), anything())).thenResolve(
when(appShell.showInformationMessage(anything(), anything(), anything(), anything())).thenResolve(
Common.ok() as any
);

await scriptSourceProvider.getWidgetScriptSource('HelloWorld', '1');

// Confirm message was displayed.
verify(
appShell.showInformationMessage(DataScience.useCDNForWidgets(), Common.ok(), Common.cancel())
).once();

// Confirm settings were updated.
verify(
configService.updateSetting(
'dataScience.widgets.localConnectionScriptSources',
deepEqual(['jsdelivr.com', 'unpkg.com', 'localPythonEnvironment']),
undefined,
ConfigurationTarget.Global
appShell.showInformationMessage(
DataScience.useCDNForWidgets(),
Common.ok(),
Common.cancel(),
Common.doNotShowAgain()
)
).once();
// Confirm settings were updated.
verify(userSelectedOkOrDoNotShowAgainInPrompt.updateValue(true)).once();
verify(
configService.updateSetting(
'dataScience.widgets.remoteConnectionScriptSources',
deepEqual(['jsdelivr.com', 'unpkg.com', 'remoteJupyterServer']),
'dataScience.widgetScriptSources',
deepEqual(['jsdelivr.com', 'unpkg.com']),
undefined,
ConfigurationTarget.Global
)
Expand Down Expand Up @@ -217,76 +247,10 @@ suite('Data Science - ipywidget - Widget Script Source Provider', () => {
localOrRemoteSource.reset();
cdnSource.reset();
localOrRemoteSource.resolves([{ moduleName: 'moduleLocal', scriptUri: '1', source: 'local' }]);
settings.datascience.widgetScriptSources = ['jsdelivr.com', 'unpkg.com'];
settings.datascience.widgetScriptSources = [];
onDidChangeWorkspaceSettings.fire({ affectsConfiguration: () => true });

values = await scriptSourceProvider.getWidgetScriptSources();

// Confirm Local source is given preference or remote was given preference.
// Ie. we shoudln't have got the module from a CDN.
assert.deepEqual(values, [{ moduleName: 'moduleLocal', scriptUri: '1', source: 'local' }]);
assert.isTrue(localOrRemoteSource.calledOnce);
assert.isFalse(cdnSource.calledOnce);

// 3. Now removing local and remote jupyter, meaning we always want to get from CDN.
localOrRemoteSource.reset();
cdnSource.reset();
cdnSource.resolves([{ moduleName: 'moduleCDN', scriptUri: '1', source: 'cdn' }]);
settings.datascience.widgetScriptSources = ['jsdelivr.com'];
onDidChangeWorkspaceSettings.fire({ affectsConfiguration: () => true });

values = await scriptSourceProvider.getWidgetScriptSources();

// 2. Confirm we got the module from CDN and not from local/remote.
assert.deepEqual(values, [{ moduleName: 'moduleCDN', scriptUri: '1', source: 'cdn' }]);
assert.isFalse(localOrRemoteSource.calledOnce);
assert.isTrue(cdnSource.calledOnce);
});
test('Widget source should respect changes to configuration settings', async () => {
// 1. Search CDN then local/remote juptyer.
settings.datascience.widgetScriptSources = ['jsdelivr.com', 'unpkg.com'];
const localOrRemoteSource = localLaunch
? sinon.stub(LocalWidgetScriptSourceProvider.prototype, 'getWidgetScriptSource')
: sinon.stub(RemoteWidgetScriptSourceProvider.prototype, 'getWidgetScriptSource');
const cdnSource = sinon.stub(CDNWidgetScriptSourceProvider.prototype, 'getWidgetScriptSource');
cdnSource.resolves({ moduleName: 'moduleCDN', scriptUri: '1', source: 'cdn' });

scriptSourceProvider.initialize();
let value = await scriptSourceProvider.getWidgetScriptSource('', '');

// 1. Confirm CDN was given preference.
assert.deepEqual(value, { moduleName: 'moduleCDN', scriptUri: '1', source: 'cdn' });
assert.isFalse(localOrRemoteSource.calledOnce);
assert.isTrue(cdnSource.calledOnce);

// 2. Update settings to use local python environment or remote jupyter in case of remote connection.
localOrRemoteSource.reset();
cdnSource.reset();
localOrRemoteSource.resolves({ moduleName: 'moduleLocal', scriptUri: '1', source: 'local' });
settings.datascience.widgetScriptSources = ['jsdelivr.com', 'unpkg.com'];
onDidChangeWorkspaceSettings.fire({ affectsConfiguration: () => true });

value = await scriptSourceProvider.getWidgetScriptSource('', '');

// Confirm Local source is given preference or remote was given preference.
// Ie. we shoudln't have got the module from a CDN.
assert.deepEqual(value, { moduleName: 'moduleLocal', scriptUri: '1', source: 'local' });
assert.isTrue(localOrRemoteSource.calledOnce);
assert.isFalse(cdnSource.calledOnce);

// 3. Now removing local and remote jupyter, meaning we always want to get from CDN.
localOrRemoteSource.reset();
cdnSource.reset();
cdnSource.resolves({ moduleName: 'moduleCDN', scriptUri: '1', source: 'cdn' });
settings.datascience.widgetScriptSources = ['jsdelivr.com'];
onDidChangeWorkspaceSettings.fire({ affectsConfiguration: () => true });

value = await scriptSourceProvider.getWidgetScriptSource('', '');

// 2. Confirm we got the module from CDN and not from local/remote.
assert.deepEqual(value, { moduleName: 'moduleCDN', scriptUri: '1', source: 'cdn' });
assert.isFalse(localOrRemoteSource.calledOnce);
assert.isTrue(cdnSource.calledOnce);
});
test('Widget source should support fall back search', async () => {
// 1. Search CDN and if that fails then get from local/remote.
Expand All @@ -308,22 +272,21 @@ suite('Data Science - ipywidget - Widget Script Source Provider', () => {
// Confirm we first searched CDN before going to local/remote.
cdnSource.calledBefore(localOrRemoteSource);
});
test('Widget sources from CDN should be given prefernce', async function () {
if (!localLaunch) {
return this.skip();
}
test('Widget sources from CDN should be given prefernce', async () => {
settings.datascience.widgetScriptSources = ['jsdelivr.com', 'unpkg.com'];
const localSource = sinon.stub(LocalWidgetScriptSourceProvider.prototype, 'getWidgetScriptSources');
const localOrRemoteSource = localLaunch
? sinon.stub(LocalWidgetScriptSourceProvider.prototype, 'getWidgetScriptSources')
: sinon.stub(RemoteWidgetScriptSourceProvider.prototype, 'getWidgetScriptSources');
const cdnSource = sinon.stub(CDNWidgetScriptSourceProvider.prototype, 'getWidgetScriptSources');

localSource.resolves([]);
localOrRemoteSource.resolves([]);
cdnSource.resolves([{ moduleName: 'module1', scriptUri: '1', source: 'cdn' }]);

scriptSourceProvider.initialize();
const values = await scriptSourceProvider.getWidgetScriptSources();

assert.deepEqual(values, [{ moduleName: 'module1', scriptUri: '1', source: 'cdn' }]);
assert.isFalse(localSource.calledOnce);
assert.isFalse(localOrRemoteSource.calledOnce);
assert.isTrue(cdnSource.calledOnce);
});
});
Expand Down