8000 Use Promise.all to fetch files part of py-config (#1322) · SamuelDelmonte/pyscript-1@9fedfe3 · GitHub 8000
[go: up one dir, main page]
Skip to content

Commit 9fedfe3

Browse files
hoodmanehoodmane
authored
Use Promise.all to fetch files part of py-config (pyscript#1322)
This is a first step towards loading more stuff simultaneously rather than sequentially. The functional part of this is pretty small: call `calculateFetchPaths` and then `Promise.all(fetchPaths.map(loadFileFromURL));`. I also transposed the return type of `calculateFetchPaths` since it's more convenient to consume this way. I redid the logic in `calculateFetchPaths` a bit. I renamed `src/plugins/fetch.ts` to `calculateFetchPaths.ts` since the file performs no fetching. I also renamed `loadFromFile` to `loadFileFromURL`.
1 parent 26f0724 commit 9fedfe3

File tree

6 files changed

+80
-85
lines changed

6 files cha 8000 nged

+80
-85
lines changed

‎pyscriptjs/src/main.ts

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { PluginManager, define_custom_element, Plugin } from './plugin';
88
import { make_PyScript, initHandlers, mountElements } from './components/pyscript';
99
import { getLogger } from './logger';
1010
import { showWarning, globalExport, createLock } from './utils';
11-
import { calculatePaths } from './plugins/fetch';
11+
import { calculateFetchPaths } from './plugins/calculateFetchPaths';
1212
import { createCustomElements } from './components/elements';
1313
import { UserError, ErrorCode, _createAlertBanner } from './exceptions';
1414
import { type Stdio, StdioMultiplexer, DEFAULT_STDIO } from './stdio';
@@ -304,34 +304,33 @@ export class PyScriptApp {
304304
pyscript._install_deprecated_globals_2022_12_1(globals())
305305
`);
306306

307-
if (this.config.packages) {
308-
logger.info('Packages to install: ', this.config.packages);
309-
await interpreter._remote.installPackage(this.config.packages);
310-
}
311-
await this.fetchPaths(interpreter);
307+
await Promise.all([this.installPackages(), this.fetchPaths(interpreter)]);
312308

313309
//This may be unnecessary - only useful if plugins try to import files fetch'd in fetchPaths()
314310
await interpreter._remote.invalidate_module_path_cache();
315311
// Finally load plugins
316312
await this.fetchUserPlugins(interpreter);
317313
}
318314

319-
async fetchPaths(interpreter: InterpreterClient) {
320-
// XXX this can be VASTLY improved: for each path we need to fetch a
321-
// URL and write to the virtual filesystem: pyodide.loadFromFile does
322-
// it in Python, which means we need to have the interpreter
323-
// initialized. But we could easily do it in JS in parallel with the
324-
// download/startup of pyodide.
325-
const [paths, fetchPaths] = calculatePaths(this.config.fetch);
326-
logger.info('Paths to write: ', paths);
327-
logger.info('Paths to fetch: ', fetchPaths);
328-
for (let i = 0; i < paths.length; i++) {
329-
logger.info(` fetching path: ${fetchPaths[i]}`);
330-
331-
// Exceptions raised from here will create an alert banner
332-
await interpreter._remote.loadFromFile(paths[i], fetchPaths[i]);
315+
async installPackages() {
316+
if (!this.config.packages) {
317+
return;
333318
}
334-
logger.info('All paths fetched');
319+
logger.info('Packages to install: ', this.config.packages);
320+
await this.interpreter._remote.installPackage(this.config.packages);
321+
}
322+
323+
async fetchPaths(interpreter: InterpreterClient) {
324+
// TODO: start fetching before interpreter initialization
325+
const paths = calculateFetchPaths(this.config.fetch);
326+
logger.info('Fetching urls:', paths.map(({ url }) => url).join(', '));
327+
await Promise.all(
328+
paths.map(async ({ path, url }) => {
329+
await interpreter._remote.loadFileFromURL(path, url);
330+
logger.info(` Fetched ${url} ==> ${path}`);
331+
}),
332+
);
333+
logger.info('Fetched all paths');
335334
}
336335

337336
/**
@@ -408,7 +407,7 @@ export class PyScriptApp {
408407
const pathArr = filePath.split('/');
409408
const filename = pathArr.pop();
410409
// TODO: Would be probably be better to store plugins somewhere like /plugins/python/ or similar
411-
await interpreter._remote.loadFromFile(filename, filePath);
410+
await interpreter._remote.loadFileFromURL(filename, filePath);
412411

413412
//refresh module cache before trying to import module files into interpreter
414413
await interpreter._remote.invalidate_module_path_cache();

‎pyscriptjs/src/plugins/calculateFetchPaths.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { joinPaths } from '../utils';
2+
import { FetchConfig } from '../pyconfig';
3+
import { UserError, ErrorCode } from '../exceptions';
4+
5+
export function calculateFetchPaths(fetch_cfg: FetchConfig[]): { url: string; path: string }[] {
6+
for (const { files, to_file, from = '' } of fetch_cfg) {
7+
if (files !== undefined && to_file !== undefined) {
8+
throw new UserError(ErrorCode.BAD_CONFIG, `Cannot use 'to_file' and 'files' parameters together!`);
9+
}
10+
if (files === undefined && to_file === undefined && from.endsWith('/')) {
11+
throw new UserError(
12+
ErrorCode.BAD_CONFIG,
13+
`Couldn't determine the filename from the path ${from}, please supply 'to_file' parameter.`,
14+
);
15+
}
16+
}
17+
18+
return fetch_cfg.flatMap(function ({ from = '', to_folder = '.', to_file, files }) {
19+
if (files !== undefined) {
20+
return files.map(file => ({ url: joinPaths([from, file]), path: joinPaths([to_folder, file]) }));
21+
}
22+
const filename = to_file || from.slice(1 + from.lastIndexOf('/'));
23+
const to_path = joinPaths([to_folder, filename]);
24+
return [{ url: from, path: to_path }];
25+
});
26+
}

‎pyscriptjs/src/plugins/fetch.ts

Lines changed: 0 additions & 37 deletions
This file was deleted.

‎pyscriptjs/src/remote_interpreter.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -226,11 +226,11 @@ export class RemoteInterpreter extends Object {
226226
/**
227227
*
228228
* @param path : the path in the filesystem
229-
* @param fetch_path : the path to be fetched
229+
* @param url : the url to be fetched
230230
*
231-
* Given a file available at `fetch_path` URL (eg:
232-
* `http://dummy.com/hi.py`), the function downloads the file and saves it
233-
* to the `path` (eg: `a/b/c/foo.py`) on the FS.
231+
* Given a file available at `url` URL (eg: `http://dummy.com/hi.py`), the
232+
* function downloads the file and saves it to the `path` (eg:
233+
* `a/b/c/foo.py`) on the FS.
234234
*
235235
* Example usage: await loadFromFile(`a/b/c/foo.py`,
236236
* `http://dummy.com/hi.py`)
@@ -246,13 +246,13 @@ export class RemoteInterpreter extends Object {
246246
* in `/home/pyodide/a/b.py`, `../a/b.py` will be placed into `/home/a/b.py`
247247
* and `/a/b.py` will be placed into `/a/b.py`.
248248
*/
249-
async loadFromFile(path: string, fetch_path: string): Promise<void> {
249+
async loadFileFromURL(path: string, url: string): Promise<void> {
250250
path = this.PATH_FS.resolve(path);
251251
const dir: string = this.PATH.dirname(path);
252252
this.FS.mkdirTree(dir);
253253

254254
// `robustFetch` checks for failures in getting a response
255-
const response = await robustFetch(fetch_path);
255+
const response = await robustFetch(url);
256256
const buffer = await response.arrayBuffer();
257257
const data = new Uint8Array(buffer);
258258

‎pyscriptjs/src/utils.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ export function joinPaths(parts: string[], separator = '/') {
8080
.map(function (part) {
8181
return part.trim().replace(/(^[/]*|[/]*$)/g, '');
8282
})
83-
.filter(p => p !== '')
83+
.filter(p => p !== '' && p !== '.')
8484
.join(separator || '/');
8585
if (parts[0].startsWith('/')) {
8686
return '/' + res;

‎pyscriptjs/tests/unit/fetch_plugin.test.ts renamed to ‎pyscriptjs/tests/unit/calculateFetchPaths.test.ts

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,55 +1,62 @@
1-
import { calculatePaths } from '../../src/plugins/fetch';
1+
import { calculateFetchPaths } from '../../src/plugins/calculateFetchPaths';
22
import { FetchConfig } from '../../src/pyconfig';
33

44
describe('CalculateFetchPaths', () => {
55
it('should calculate paths when only from is provided', () => {
66
const fetch_cfg: FetchConfig[] = [{ from: 'http://a.com/data.csv' }];
7-
const [paths, fetchPaths] = calculatePaths(fetch_cfg);
8-
expect(paths).toStrictEqual(['./data.csv']);
9-
expect(fetchPaths).toStrictEqual(['http://a.com/data.csv']);
7+
const res = calculateFetchPaths(fetch_cfg);
8+
expect(res).toStrictEqual([{ url: 'http://a.com/data.csv', path: 'data.csv' }]);
109
});
1110

1211
it('should calculate paths when only files is provided', () => {
13-
const fetch_cfg: FetchConfig[] = [{ files: ['foo/__init__.py', 'foo/mod.py'] }];
14-
const [paths, fetchPaths] = calculatePaths(fetch_cfg);
15-
expect(paths).toStrictEqual(['./foo/__init__.py', './foo/mod.py']);
16-
expect(fetchPaths).toStrictEqual(['foo/__init__.py', 'foo/mod.py']);
12+
const fetch_cfg: FetchConfig[] = [{ files: ['foo/__init__.py', 'foo/mod.py', 'foo2/mod.py'] }];
13+
const res = calculateFetchPaths(fetch_cfg);
14+
expect(res).toStrictEqual([
15+
{ url: 'foo/__init__.py', path: 'foo/__init__.py' },
16+
{ url: 'foo/mod.py', path: 'foo/mod.py' },
17+
{ url: 'foo2/mod.py', path: 'foo2/mod.py' },
18+
]);
1719
});
1820

1921
it('should calculate paths when files and to_folder is provided', () => {
2022
const fetch_cfg: FetchConfig[] = [{ files: ['foo/__init__.py', 'foo/mod.py'], to_folder: '/my/lib/' }];
21-
const [paths, fetchPaths] = calculatePaths(fetch_cfg);
22-
expect(paths).toStrictEqual(['/my/lib/foo/__init__.py', '/my/lib/foo/mod.py']);
23-
expect(fetchPaths).toStrictEqual(['foo/__init__.py', 'foo/mod.py']);
23+
const res = calculateFetchPaths(fetch_cfg);
24+
expect(res).toStrictEqual([
25+
{ url: 'foo/__init__.py', path: '/my/lib/foo/__init__.py' },
26+
{ url: 'foo/mod.py', path: '/my/lib/foo/mod.py' },
27+
]);
2428
});
2529

2630
it('should calculate paths when from and files and to_folder is provided', () => {
2731
const fetch_cfg: FetchConfig[] = [
2832
{ from: 'http://a.com/download/', files: ['foo/__init__.py', 'foo/mod.py'], to_folder: '/my/lib/' },
2933
];
30-
const [paths, fetchPaths] = calculatePaths(fetch_cfg);
31-
expect(paths).toStrictEqual(['/my/lib/foo/__init__.py', '/my/lib/foo/mod.py']);
32-
expect(fetchPaths).toStrictEqual(['http://a.com/download/foo/__init__.py', 'http://a.com/download/foo/mod.py']);
34+
const res = calculateFetchPaths(fetch_cfg);
35+
expect(res).toStrictEqual([
36+
{ url: 'http://a.com/download/foo/__init__.py', path: '/my/lib/foo/__init__.py' },
37+
{ url: 'http://a.com/download/foo/mod.py', path: '/my/lib/foo/mod.py' },
38+
]);
3339
});
3440

3541
it("should error out while calculating paths when filename cannot be determined from 'from'", () => {
3642
const fetch_cfg: FetchConfig[] = [{ from: 'http://google.com/', to_folder: '/tmp' }];
37-
expect(() => calculatePaths(fetch_cfg)).toThrowError(
43+
expect(() => calculateFetchPaths(fetch_cfg)).toThrowError(
3844
"Couldn't determine the filename from the path http://google.com/",
3945
);
4046
});
4147

4248
it('should calculate paths when to_file is explicitly supplied', () => {
4349
const fetch_cfg: FetchConfig[] = [{ from: 'http://a.com/data.csv?version=1', to_file: 'pkg/tmp/data.csv' }];
44-
const [paths, fetchPaths] = calculatePaths(fetch_cfg);
45-
expect(paths).toStrictEqual(['./pkg/tmp/data.csv']);
46-
expect(fetchPaths).toStrictEqual(['http://a.com/data.csv?version=1']);
50+
const res = calculateFetchPaths(fetch_cfg);
51+
expect(res).toStrictEqual([{ path: 'pkg/tmp/data.csv', url: 'http://a.com/data.csv?version=1' }]);
4752
});
4853

4954
it('should error out when both to_file and files parameters are provided', () => {
5055
const fetch_cfg: FetchConfig[] = [
5156
{ from: 'http://a.com/data.csv?version=1', to_file: 'pkg/tmp/data.csv', files: ['a.py', 'b.py'] },
5257
];
53-
expect(() => calculatePaths(fetch_cfg)).toThrowError("Cannot use 'to_file' and 'files' parameters together!");
58+
expect(() => calculateFetchPaths(fetch_cfg)).toThrowError(
59+
"Cannot use 'to_file' and 'files' parameters together!",
60+
);
5461
});
5562
});

0 commit comments

Comments
 (0)
0