Skip to content
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

use latest Jupyterlite with programatic options #612

Merged
merged 9 commits into from
Apr 10, 2023
Merged

Conversation

stevejpurves
Copy link
Collaborator

The purpose of this PR is to update to using the latest juptyerlite codebase, this involves adjusting to the new organisation of the @juptyerlite/pyodide-kernel and taking steps to specify the location of the wheels to load. This should be a big improvement over the previous implementation once in place, and the objective is to be able to load the latest wheels from CDN (possibly direct from unpkg.com).

@stevejpurves stevejpurves marked this pull request as draft April 4, 2023 14:09
pipliteUrls: ['https://unpkg.com/@jupyterlite/pyodide-kernel@0.0.6/pypi/all.json'],
},
}),
);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we should be able to set the litePluginSettings here? and this would be ahead of the @jupyterlite/pyodide-kernel-extension being activated, so it should be able to pick up on these?

Copy link
Contributor

Choose a reason for hiding this comment

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

so it should be able to pick up on these?

Yes I would assume so.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

cool - and yes I am seeing some impact of setting this but it's not loading the piplite wheel from unpkg for some reason.

i.e. with setting the litePluginSettings here, on initializing the kernel two of the wheels are loaded from unpkg.com 👏 but it still looks to the /pypi/all.json locally and still loads piplite from there 🤔

image

Copy link
Collaborator Author

@stevejpurves stevejpurves Apr 4, 2023

Choose a reason for hiding this comment

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

ok so this kind of makes sense as litePluginSettings is setting the pipliteUrls i.e. where piplite will load it's wheels from -- we need to also configure something separate to direct where the piplite wheel itself will be loaded from...?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

opened jupyterlite/pyodide-kernel#42 to address this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have also started looking at the remote federated module loading, which I think I'll start in a separate PR on top of this one.

I'm just wondering though, if we put that remote loading in place it will immediately supercede this arrangement of build-time including the server extension while picking up the wheels from online - so it's just better to move to that now. Then AFAIK this will mean that the kernel can be specified by option at runtime, and also be interchangeable (i.e. we can load other xeus kernels too)

@stevejpurves
Copy link
Collaborator Author

stevejpurves commented Apr 4, 2023

currently when trying to run jlite.html in the simple demo, we hit this problem during kernel initialization:

image

I was expecting that setting the litePluginOptions appropriately would address this but we're not seeing even a network call being made to the all.json.

cc @jtpio does this seem like a reasonable approach?

@github-actions
Copy link
Contributor

github-actions bot commented Apr 4, 2023

PR Preview Action 8eaa763
Preview removed because the pull request was closed.
2023-04-10 20:55 UTC

@jtpio
Copy link
Contributor

jtpio commented Apr 4, 2023

The screenshot suggests the name is malformed, but they seem to look good in the npm package: https://www.npmjs.com/package/@jupyterlite/pyodide-kernel?activeTab=code

image

If you use the @jupyterlite/pyodide-kernel directly then it should be possible to import them from /pypi yes.

Maybe we could also check the Web Worker is correctly getting the wheel info.

@stevejpurves
Copy link
Collaborator Author

@jtpio yes the wheels look good in the package - what I am seeing though is webpack is adding them to my distribution and replacing the names with hashes, so my dist folder contains randomly hashed .whl files.

@jtpio
Copy link
Contributor

jtpio commented Apr 4, 2023

so my dist folder contains randomly hashed .whl files.

You might need to use the Asset Modules with a generator like here: https://github.com/jupyterlite/pyodide-kernel/blob/01ef2f5ad3c468205d78dddeffa2990c0bd4eee9/packages/pyodide-kernel-extension/webpack.config.js#L8-L12

@stevejpurves
Copy link
Collaborator Author

ok - so status after that last commit:

  • build working and it's simpler than before!
    • service worker gets installed automatically 👏💪
    • I no longer have to change the location of the wheels output by webpack, pypi folder appears in the right place in the dist folder
  • I'm specifying lite plugin options in the simple demo, but that is only picking up 2 of the 3 wheels from unpkg (see comment above)

@stevejpurves stevejpurves marked this pull request as ready for review April 10, 2023 20:40
@stevejpurves
Copy link
Collaborator Author

stevejpurves commented Apr 10, 2023

ok I have changed the pageconfig to set the litePluginOptions based on an optional argument in the start server function. As it is this would allow runtime changes to point piplite to look for wheels at a remote location. This means that the iolie wheel itself still needs to be hosted locally but if jupyterlite/pyodide-kernel#42 is merged and released then all wheels can hosted remotely.

@stevejpurves stevejpurves merged commit 8eaa763 into main Apr 10, 2023
@stevejpurves stevejpurves deleted the feat/jlite-latest branch April 10, 2023 20:51
@stevejpurves
Copy link
Collaborator Author

stevejpurves commented Apr 10, 2023

merged #612 which addresses a lot of this but we still need some testing to see if that last todo on startup signalling has been achieved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants