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

Supporting arbitrary indexURL #4

Closed
rth opened this issue Sep 24, 2022 · 2 comments
Closed

Supporting arbitrary indexURL #4

rth opened this issue Sep 24, 2022 · 2 comments

Comments

@rth
Copy link
Member

rth commented Sep 24, 2022

So currently there is autoCdnIndexUrl option but it could also be useful to be able to pass an arbitrary indexURL, as we want to encourage people to be able to do their own deployment of the subset of packages they need.

For instance, in some cases, we may want to test the dev channel https://pyodide.org/en/stable/usage/downloading-and-deploying.html#cdn

Also how does one specify the Pyodide version to use? That would be via the version of the pyodide npm package, right? Do we expect this to work with just the latest release or a range of releases (say starting from the current one)?

Asking lots of questions as a user :)

@mneil
Copy link
Collaborator

mneil commented Sep 24, 2022

The pyodide version is whatever the user has installed. I opened a PR to note a minimum version of pyodide that works with the plugin. The plugin should support all versions going forward.

I'll open a separate PR with the ability to set the CDN url. I also updated the docs on how the autoCdnIndexUrl differs from setting the IndexURL in loadPyodide

mneil added a commit that referenced this issue Sep 24, 2022
Change due to pre-release and open issue #4 noting that users may
want to change the url. Having the value be a boolean would mean
setting the url would require a third option.

Opted to set the value as a string instead and default to the
release url. If users want to change it they can set it to whatever
url they would like.
@mneil
Copy link
Collaborator

mneil commented Sep 24, 2022

On second thought @rth please look at #5 and let me know what you think. Rather than having autoCdnUrl and a boolean value I changed it to micropipCdnUrl with the value as a string. Same default, but now you can change the url. Set the url to "" to use the default from pyodide which is to use "indexUrl".

The reason for this existing at all is that indexUrl in pyodide tries to look up the url where pyodide.js was served. That ends up being localhost or whatever the user's url is when they use pyodide and this plugin to serve pyodide on a webpage. If you then try to use micropip it looks at the same url to find micropip and other pip packages. However, none of those packages are downloaded with the pyodide package.

Setting loadPyodide indexUrl can work but then it serves all files from the indexUrl and ignores all the asm / data packages in the pyodide package. What I wanted to allow is for users to use all files from the pyodide npm package and a different URL only for micropip.

Maybe this is something pyodide core could do better at. But for now supporting it in this plugin for webpack users is fairly trivial.

I have not released this to npm yet. I wouldn't make such a drastic change if I already had. I'll wait to hear feedback on this change before finally releasing to npm.

mneil added a commit that referenced this issue Sep 24, 2022
Change due to pre-release and open issue #4 noting that users may
want to change the url. Having the value be a boolean would mean
setting the url would require a third option.

Opted to set the value as a string instead and default to the
release url. If users want to change it they can set it to whatever
url they would like.
@mneil mneil closed this as completed Nov 21, 2022
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

No branches or pull requests

2 participants