Skip to content

Commit bf6a523

Browse files
rchiodorchiodo
authored
Port two fixes to the release branch (#11337)
* Change CDN files to download locally (#11286) * Working downloader * Tests passing * Add more tests * Add retry test * Fix build errors. Not sure why not happening locally. * Fix unit tests * Fix azure ml on windows Rework error checking to use status * Fix sonar error * Add some more descriptive comments * More comments * Refactor some code * Make sure to not use exists as we're downloading anyway * Fix container timeout from firing unnecessarily (#11336) * Fix container timeout from firing unnecessarily * Check for undefined for timer * Update changelog
1 parent 69d2691 commit bf6a523

13 files changed

Lines changed: 512 additions & 155 deletions

‎CHANGELOG.md‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,68 @@
11
# Changelog
22

3+
## 2020.4.1 (27 April 2020)
4+
5+
### Fixes
6+
7+
1. Fix issue where downloading ipywidgets from the CDN might be busy.
8+
([#11274](https://github.com/Microsoft/vscode-python/issues/11274))
9+
1. Error: Timeout is shown after running any widget more than once.
10+
([#11334](https://github.com/Microsoft/vscode-python/issues/11334))
11+
12+
### Thanks
13+
14+
Thanks to the following projects which we fully rely on to provide some of
15+
our features:
16+
17+
- [debugpy](https://pypi.org/project/debugpy/)
18+
- [isort](https://pypi.org/project/isort/)
19+
- [jedi](https://pypi.org/project/jedi/)
20+
and [parso](https://pypi.org/project/parso/)
21+
- [Microsoft Python Language Server](https://github.com/microsoft/python-language-server)
22+
- [ptvsd](https://pypi.org/project/ptvsd/)
23+
- [exuberant ctags](http://ctags.sourceforge.net/) (user-installed)
24+
- [rope](https://pypi.org/project/rope/) (user-installed)
25+
26+
Also thanks to the various projects we provide integrations with which help
27+
make this extension useful:
28+
29+
- Debugging support:
30+
[Django](https://pypi.org/project/Django/),
31+
[Flask](https://pypi.org/project/Flask/),
32+
[gevent](https://pypi.org/project/gevent/),
33+
[Jinja](https://pypi.org/project/Jinja/),
34+
[Pyramid](https://pypi.org/project/pyramid/),
35+
[PySpark](https://pypi.org/project/pyspark/),
36+
[Scrapy](https://pypi.org/project/Scrapy/),
37+
[Watson](https://pypi.org/project/Watson/)
38+
- Formatting:
39+
[autopep8](https://pypi.org/project/autopep8/),
40+
[black](https://pypi.org/project/black/),
41+
[yapf](https://pypi.org/project/yapf/)
42+
- Interpreter support:
43+
[conda](https://conda.io/),
44+
[direnv](https://direnv.net/),
45+
[pipenv](https://pypi.org/project/pipenv/),
46+
[pyenv](https://github.com/pyenv/pyenv),
47+
[venv](https://docs.python.org/3/library/venv.html#module-venv),
48+
[virtualenv](https://pypi.org/project/virtualenv/)
49+
- Linting:
50+
[bandit](https://pypi.org/project/bandit/),
51+
[flake8](https://pypi.org/project/flake8/),
52+
[mypy](https://pypi.org/project/mypy/),
53+
[prospector](https://pypi.org/project/prospector/),
54+
[pylint](https://pypi.org/project/pylint/),
55+
[pydocstyle](https://pypi.org/project/pydocstyle/),
56+
[pylama](https://pypi.org/project/pylama/)
57+
- Testing:
58+
[nose](https://pypi.org/project/nose/),
59+
[pytest](https://pypi.org/project/pytest/),
60+
[unittest](https://docs.python.org/3/library/unittest.html#module-unittest)
61+
62+
And finally thanks to the [Python](https://www.python.org/) development team and
63+
community for creating a fantastic programming language and community to be a
64+
part of!
65+
366
## 2020.4.0 (20 April 2020)
467

568
### Enhancements

‎src/client/datascience/constants.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,7 @@ export namespace Identifiers {
404404
export const InteractiveWindowIdentity = 'history://EC155B3B-DC18-49DC-9E99-9A948AA2F27B';
405405
export const InteractiveWindowIdentityScheme = 'history';
406406
export const DefaultCodeCellMarker = '# %%';
407+
export const DefaultCommTarget = 'jupyter.widget';
407408
}
408409

409410
export namespace CodeSnippits {

‎src/client/datascience/ipywidgets/cdnWidgetScriptSourceProvider.ts‎

Lines changed: 183 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,25 @@
33

44
'use strict';
55

6-
import { traceWarning } from '../../common/logger';
6+
import * as fs from 'fs-extra';
7+
import { sha256 } from 'hash.js';
8+
import * as path from 'path';
9+
import request from 'request';
10+
import { Uri } from 'vscode';
11+
import { traceError, traceInfo } from '../../common/logger';
12+
import { IFileSystem, TemporaryFile } from '../../common/platform/types';
713
import { IConfigurationService, IHttpClient, WidgetCDNs } from '../../common/types';
8-
import { StopWatch } from '../../common/utils/stopWatch';
9-
import { sendTelemetryEvent } from '../../telemetry';
10-
import { Telemetry } from '../constants';
14+
import { createDeferred, sleep } from '../../common/utils/async';
15+
import { ILocalResourceUriConverter } from '../types';
1116
import { IWidgetScriptSourceProvider, WidgetScriptSource } from './types';
1217

1318
// Source borrowed from https://github.com/jupyter-widgets/ipywidgets/blob/54941b7a4b54036d089652d91b39f937bde6b6cd/packages/html-manager/src/libembed-amd.ts#L33
1419
const unpgkUrl = 'https://unpkg.com/';
1520
const jsdelivrUrl = 'https://cdn.jsdelivr.net/npm/';
21+
22+
// tslint:disable: no-var-requires no-require-imports
23+
const sanitize = require('sanitize-filename');
24+
1625
function moduleNameToCDNUrl(cdn: string, moduleName: string, moduleVersion: string) {
1726
let packageName = moduleName;
1827
let fileName = 'index'; // default filename
@@ -29,6 +38,16 @@ function moduleNameToCDNUrl(cdn: string, moduleName: string, moduleVersion: stri
2938
fileName = moduleName.substr(index + 1);
3039
packageName = moduleName.substr(0, index);
3140
}
41+
if (cdn === jsdelivrUrl) {
42+
// Js Delivr doesn't support ^ in the version. It needs an exact version
43+
if (moduleVersion.startsWith('^')) {
44+
moduleVersion = moduleVersion.slice(1);
45+
}
46+
// Js Delivr also needs the .js file on the end.
47+
if (!fileName.endsWith('.js')) {
48+
fileName = fileName.concat('.js');
49+
}
50+
}
3251
return `${cdn}${packageName}@${moduleVersion}/dist/${fileName}`;
3352
}
3453

@@ -52,40 +71,177 @@ export class CDNWidgetScriptSourceProvider implements IWidgetScriptSourceProvide
5271
const settings = this.configurationSettings.getSettings(undefined);
5372
return settings.datascience.widgetScriptSources;
5473
}
55-
public static validUrls = new Map<string, boolean>();
74+
private cache = new Map<string, WidgetScriptSource>();
5675
constructor(
5776
private readonly configurationSettings: IConfigurationService,
58-
private readonly httpClient: IHttpClient
77+
private readonly httpClient: IHttpClient,
78+
private readonly localResourceUriConverter: ILocalResourceUriConverter,
79+
private readonly fileSystem: IFileSystem
5980
) {}
6081
public dispose() {
61-
// Noop.
82+
this.cache.clear();
6283
}
6384
public async getWidgetScriptSource(moduleName: string, moduleVersion: string): Promise<WidgetScriptSource> {
64-
const cdns = [...this.cdnProviders];
65-
while (cdns.length) {
66-
const cdn = cdns.shift();
67-
const cdnBaseUrl = getCDNPrefix(cdn);
68-
if (!cdnBaseUrl || !cdn) {
69-
continue;
85+
// First see if we already have it downloaded.
86+
const key = this.getModuleKey(moduleName, moduleVersion);
87+
const diskPath = path.join(this.localResourceUriConverter.rootScriptFolder.fsPath, key, 'index.js');
88+
let cached = this.cache.get(key);
89+
let tempFile: TemporaryFile | undefined;
90+
91+
// Might be on disk, try there first.
92+
if (!cached) {
93+
if (diskPath && (await this.fileSystem.fileExists(diskPath))) {
94+
const scriptUri = (await this.localResourceUriConverter.asWebviewUri(Uri.file(diskPath))).toString();
95+
cached = { moduleName, scriptUri, source: 'cdn' };
96+
this.cache.set(key, cached);
7097
}
71-
const scriptUri = moduleNameToCDNUrl(cdnBaseUrl, moduleName, moduleVersion);
72-
const exists = await this.getUrlForWidget(cdn, scriptUri);
73-
if (exists) {
74-
return { moduleName, scriptUri, source: 'cdn' };
98+
}
99+
100+
// If still not found, download it.
101+
if (!cached) {
102+
try {
103+
// Make sure the disk path directory exists. We'll be downloading it to there.
104+
await this.fileSystem.createDirectory(path.dirname(diskPath));
105+
106+
// Then get the first one that returns.
107+
tempFile = await this.downloadFastestCDN(moduleName, moduleVersion);
108+
if (tempFile) {
109+
// Need to copy from the temporary file to our real file (note: VSC filesystem fails to copy so just use straight file system)
110+
await fs.copyFile(tempFile.filePath, diskPath);
111+
112+
// Now we can generate the script URI so the local converter doesn't try to copy it.
113+
const scriptUri = (
114+
await this.localResourceUriConverter.asWebviewUri(Uri.file(diskPath))
115+
).toString();
116+
cached = { moduleName, scriptUri, source: 'cdn' };
117+
} else {
118+
cached = { moduleName };
119+
}
120+
} catch (exc) {
121+
traceError('Error downloading from CDN: ', exc);
122+
cached = { moduleName };
123+
} finally {
124+
if (tempFile) {
125+
tempFile.dispose();
126+
}
127+
}
128+
this.cache.set(key, cached);
129+
}
130+
131+
return cached;
132+
}
133+
134+
private async downloadFastestCDN(moduleName: string, moduleVersion: string) {
135+
const deferred = createDeferred<TemporaryFile | undefined>();
136+
Promise.all(
137+
// For each CDN, try to download it.
138+
this.cdnProviders.map((cdn) =>
139+
this.downloadFromCDN(moduleName, moduleVersion, cdn).then((t) => {
140+
// First one to get here wins. Meaning the first one that
141+
// returns a valid temporary file. If a request doesn't download it will
142+
// return undefined.
143+
if (!deferred.resolved && t) {
144+
deferred.resolve(t);
145+
}
146+
})
147+
)
148+
)
149+
.then((_a) => {
150+
// If after running all requests, we're still not resolved, then return empty.
151+
// This would happen if both unpkg.com and jsdelivr failed.
152+
if (!deferred.resolved) {
153+
deferred.resolve(undefined);
154+
}
155+
})
156+
.ignoreErrors();
157+
158+
// Note, we only wait until one download finishes. We don't need to wait
159+
// for everybody (hence the use of the deferred)
160+
return deferred.promise;
161+
}
162+
163+
private async downloadFromCDN(
164+
moduleName: string,
165+
moduleVersion: string,
166+
cdn: WidgetCDNs
167+
): Promise<TemporaryFile | undefined> {
168+
// First validate CDN
169+
const downloadUrl = await this.generateDownloadUri(moduleName, moduleVersion, cdn);
170+
if (downloadUrl) {
171+
// Then see if we can download the file.
172+
try {
173+
return await this.downloadFile(downloadUrl);
174+
} catch (exc) {
175+
// Something goes wrong, just fail
75176
}
76177
}
77-
traceWarning(`Widget Script not found for ${moduleName}@${moduleVersion}`);
78-
return { moduleName };
79178
}
80-
private async getUrlForWidget(cdn: string, url: string): Promise<boolean> {
81-
if (CDNWidgetScriptSourceProvider.validUrls.has(url)) {
82-
return CDNWidgetScriptSourceProvider.validUrls.get(url)!;
179+
180+
private async generateDownloadUri(
181+
moduleName: string,
182+
moduleVersion: string,
183+
cdn: WidgetCDNs
184+
): Promise<string | undefined> {
185+
const cdnBaseUrl = getCDNPrefix(cdn);
186+
if (cdnBaseUrl) {
187+
return moduleNameToCDNUrl(cdnBaseUrl, moduleName, moduleVersion);
83188
}
189+
return undefined;
190+
}
191+
192+
private getModuleKey(moduleName: string, moduleVersion: string) {
193+
return sanitize(sha256().update(`${moduleName}${moduleVersion}`).digest('hex'));
194+
}
84195

85-
const stopWatch = new StopWatch();
86-
const exists = await this.httpClient.exists(url);
87-
sendTelemetryEvent(Telemetry.DiscoverIPyWidgetNamesCDNPerf, stopWatch.elapsedTime, { cdn, exists });
88-
CDNWidgetScriptSourceProvider.validUrls.set(url, exists);
89-
return exists;
196+
private handleResponse(req: request.Request, filePath: string): Promise<boolean> {
197+
const deferred = createDeferred<boolean>();
198+
// tslint:disable-next-line: no-any
199+
const errorHandler = (e: any) => {
200+
traceError('Error downloading from CDN', e);
201+
deferred.resolve(false);
202+
};
203+
req.on('response', (r) => {
204+
if (r.statusCode === 200) {
205+
const ws = this.fileSystem.createWriteStream(filePath);
206+
r.on('error', errorHandler)
207+
.pipe(ws)
208+
.on('close', () => deferred.resolve(true));
209+
} else if (r.statusCode === 429) {
210+
// Special case busy. Sleep for 500 milliseconds
211+
sleep(500)
212+
.then(() => deferred.resolve(false))
213+
.ignoreErrors();
214+
} else {
215+
deferred.resolve(false);
216+
}
217+
}).on('error', errorHandler);
218+
return deferred.promise;
219+
}
220+
221+
private async downloadFile(downloadUrl: string): Promise<TemporaryFile | undefined> {
222+
// Create a temp file to download the results to
223+
const tempFile = await this.fileSystem.createTemporaryFile('.js');
224+
225+
// Otherwise do an http get on the url. Retry at least 5 times
226+
let retryCount = 5;
227+
let success = false;
228+
while (retryCount > 0 && !success) {
229+
let req: request.Request;
230+
try {
231+
req = await this.httpClient.downloadFile(downloadUrl);
232+
success = await this.handleResponse(req, tempFile.filePath);
233+
} catch (exc) {
234+
traceInfo(`Error downloading from ${downloadUrl}: `, exc);
235+
} finally {
236+
retryCount -= 1;
237+
}
238+
}
239+
240+
// Once we make it out, return result
241+
if (success) {
242+
return tempFile;
243+
} else {
244+
tempFile.dispose();
245+
}
90246
}
91247
}

‎src/client/datascience/ipywidgets/ipyWidgetMessageDispatcher.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { createDeferred, Deferred } from '../../common/utils/async';
1414
import { noop } from '../../common/utils/misc';
1515
import { deserializeDataViews, serializeDataViews } from '../../common/utils/serializers';
1616
import { sendTelemetryEvent } from '../../telemetry';
17-
import { Telemetry } from '../constants';
17+
import { Identifiers, Telemetry } from '../constants';
1818
import { IInteractiveWindowMapping, IPyWidgetMessages } from '../interactive-common/interactiveWindowTypes';
1919
import { INotebook, INotebookProvider, KernelSocketInformation } from '../types';
2020
import { IIPyWidgetMessageDispatcher, IPyWidgetMessage } from './types';
@@ -280,9 +280,16 @@ export class IPyWidgetMessageDispatcher implements IIPyWidgetMessageDispatcher {
280280
return;
281281
}
282282

283+
traceInfo(`Registering commtarget ${targetName}`);
283284
this.commTargetsRegistered.add(targetName);
284285
this.pendingTargetNames.delete(targetName);
285-
notebook.registerCommTarget(targetName, noop);
286+
287+
// Skip the predefined target. It should have been registered
288+
// inside the kernel on startup. However we
289+
// still need to track it here.
290+
if (targetName !== Identifiers.DefaultCommTarget) {
291+
notebook.registerCommTarget(targetName, noop);
292+
}
286293
}
287294
}
288295

0 commit comments

Comments
 (0)