Skip to content

[wip] Update to pyodide 0.22.0, use pyodide and serviceworker types #871

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.

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
merged 25 commits into from
Dec 12, 2022

Conversation

bollwyvl
Copy link
Collaborator

@bollwyvl bollwyvl commented Nov 11, 2022

References

Code changes

  • add exact-pinned pyodide to pyodide-kernel/package.json#/devDependencies
    • only ever import typed in .ts
    • is the source of truth for all other versions
  • bump to pyodide 0.22.0a3
  • remove duplicated ERRNO_CODES
  • remove use of private pyodide._module
  • update key dev deps
    • lerna 6
    • typedoc
    • typescript
    • prettier
  • refactor BroadcastChannel out of server, put in contents
  • make pyolite more resilient to missing ServiceWorker and BroadcastChannel
  • moves services.js into server/sw.ts
    • add a new webpack plugin to hoist it up to the root of the app (or the scope is wrong) and add the content hash to the file name
  • add SKIP_ESLINT env var
    • the pre-commit hooks still fail sometimes, so git commit -n it shall be!

User-facing changes

  • potentially better error messages on startup if Things Go Wrong

Backwards-incompatible changes

  • n/a
    • the sqlite3 change might have been surprising...
      • but we'll always be loading it so IPython can load

@github-actions
Copy link
Contributor

lite-badge 👈 Try it on ReadTheDocs

@@ -582,10 +462,10 @@ export class PyoliteRemoteKernel {
reject: () => void;
resolve: () => void;
} | null = null;
protected _pyodide: Pyodide.PyodideInterface = null as any;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not proud of this...

/** TODO: real typing */
protected _localPath = '';
protected _driveName = '';
protected _pyodide: any;
protected _kernel: any;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These anys still kill me. Among the tasks we should explore in #386 are:

@bollwyvl bollwyvl added kernel:pyodide related to the Pyodide webworker kernel kernel:serviceworker ServiceWorker-related challenges labels Nov 12, 2022
registration =
(await serviceWorker.getRegistration(serviceWorker.controller.scriptURL)) ||
null;
console.info('JupyterLite ServiceWorker was already registered');
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ref #751

@bollwyvl bollwyvl changed the title [wip] Update to pyodide 0.22.0 [wip] Update to pyodide 0.22.0, use pyodide and serviceworker types Nov 13, 2022
@bollwyvl
Copy link
Collaborator Author

Seems to be working on CI. Still no progress on #840, even when trying a more complex setup for handling potential race conditions on the BroadcastChannel.

Had a look at a few other things, rolled them back:

  • workbox for serviceworker
    • there's a lot here, like plugins and strategies, but I don't know how we'd get them to work inside of a simple script
    • we can however pass some config in via sw-1234.js?param=value, so there might be some things that would work well there in a more configurable way
  • bringing back importhook Add back importhook for patches #254
    • causes IPython to fail loading due to pygments tricksiness, so probably would break other things

if (
length <= 0 ||
stream.file === undefined ||
position >= (stream.file.data.length || 0)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ref

There's still something happening in the pyolite case, where doing successive writes without closing between them yields a database disk image is malformed

@jtpio jtpio added this to the 0.1.0 milestone Nov 15, 2022
@jtpio jtpio merged commit ffa115d into jupyterlite:main Dec 12, 2022
@bollwyvl bollwyvl mentioned this pull request Dec 12, 2022
2 tasks
@@ -197,6 +78,7 @@ export class PyoliteRemoteKernel {
protected async initKernel(options: IPyoliteWorkerKernel.IOptions): Promise<void> {
// from this point forward, only use piplite (but not %pip)
await this._pyodide.runPythonAsync(`
await piplite.install(['sqlite3'], keep_going=True)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will still be needed in #909

PYODIDE_CDN_URL = APP_SCHEMA_PYOLITE["properties"]["pyodideUrl"]["default"]
PYODIDE_VERSION = _js_version_to_py_version(PYODIDE_JS_VERSION)
PYODIDE_JS = PYODIDE_CDN_URL.split("/")[-1]
PYODIDE_ARCHIVE = f"pyodide-{PYODIDE_VERSION}.tar.bz2"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this name will need to change, removing -build in #909

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good let's keep track of that in #909.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file kernel:pyodide related to the Pyodide webworker kernel kernel:serviceworker ServiceWorker-related challenges maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

notebook attachment: URLs incorrectly routed to ServiceWorker
2 participants