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

Loading wheels from PyPI #147

Merged
merged 4 commits into from May 3, 2019

Conversation

@mdboom
Copy link
Member

mdboom commented Sep 7, 2018

This shows that at least it should be theoretically possible to load pure Python wheels from PyPI.

The biggest obstacle is that files.pythonhosted.org doesn't seem to allow CORS requests, so we're blocked from downloading files directly from there. There is a workaround: running a local devpi server, patched to support CORS -- details in the docs. But that's not a great solution. Perhaps files.pythonhosted.org could lift that restriction (it's possible it's just an oversight, but it could be there for good reason). Alternatively, the iodide server could grow the ability to proxy PyPI packages.

Some other missing details:

  • Package distlib as a real package, rather than vendoring
  • Provide a callback when packages have finished loading (due to browser design, they must be loaded asyncronously)
@mdboom

This comment has been minimized.

Copy link
Member Author

mdboom commented Sep 7, 2018

@mdboom mdboom changed the title WIP: Experimental support for loading packages from PyPI WIP: Experimental support for loading wheels from PyPI Sep 7, 2018
@mdboom mdboom force-pushed the mdboom:micropip branch from dc6a1e1 to f818df6 Sep 10, 2018
@di di mentioned this pull request Sep 20, 2018
@mdboom mdboom force-pushed the mdboom:micropip branch from f818df6 to 57b09db Sep 25, 2018
@mdboom

This comment has been minimized.

Copy link
Member Author

mdboom commented Sep 25, 2018

This is working now. I've configured it to use cors-anywhere.herokuapp.com to proxy the requests to pythonhosted.org. Technically you're not supposed to do this "in production", whatever that means, but maybe it's good enough for now until we get too popular. The potential is there for a man-in-the-middle attack from the proxy, but this does check the hash on the package and reject it if it doesn't match, so that should be pretty safe.

@mdboom mdboom force-pushed the mdboom:micropip branch from c895741 to f6e1514 Sep 26, 2018
@rth rth mentioned this pull request Oct 23, 2018
@mdboom mdboom mentioned this pull request Nov 1, 2018
@rth rth mentioned this pull request Nov 12, 2018
Copy link
Member

rth left a comment

Do we want to wait for PyPi to support CORS, or merge this before?

Regarding security concerns, having a proxy for pypi.io with CORS and using it as a workaround doesn't sound too bad for an experimental feature.

Is there any advantages of vendoring distlib as opposed to packaging it using the standard mechanism (even if it's a patched version)?

@mdboom

This comment has been minimized.

Copy link
Member Author

mdboom commented Jan 31, 2019

I just pinged the pypa bug to see if there's anything we can do to move it along.

I'm worried about using a third-party proxy for this. I have zero information about it, so I can't confirm or deny it's a problem, but there is at least potential for a MITM attack there. We could run our own proxy for now -- that's just "more work" :)

I just vendored distlib because I was being lazy and wanted to prove the concept. I think it's better to use it as a package like all other things. We might want to put it in the main root filesystem image though, since otherwise we'd have to load it async when using micropip.

@mdboom

This comment has been minimized.

Copy link
Member Author

mdboom commented Apr 24, 2019

I've updated this to not vendor distlib and do other nice things like return a Promise. Still stuck with the CORS proxy until those changes are deployed to pypi, though.

@mdboom mdboom force-pushed the mdboom:micropip branch from 4db400f to 142166e Apr 24, 2019
@rth

This comment has been minimized.

Copy link
Member

rth commented Apr 24, 2019

Looks like they were deployed to PyPi in pypa/conveyor#7, though I can't see the Access-Control-Allow-Origin headers when downloading a package from PyPi. I would be great to have this.

@mdboom

This comment has been minimized.

Copy link
Member Author

mdboom commented Apr 24, 2019

Does merged == deployed? Maybe there are additional steps that need to happen...

@mdboom mdboom changed the title WIP: Experimental support for loading wheels from PyPI Loading wheels from PyPI Apr 24, 2019
@mdboom mdboom requested a review from rth Apr 24, 2019
@SimonBiggs

This comment has been minimized.

Copy link

SimonBiggs commented May 2, 2019

Does merged == deployed? Maybe there are additional steps that need to happen...

From the following it seems like there was a bug causing the package content to not be accessible. This appears to now be fixed and merged:

pypa/conveyor#8

@mdboom

This comment has been minimized.

Copy link
Member Author

mdboom commented May 2, 2019

From the following it seems like there was a bug causing the package content to not be accessible.

Is that PR for the package content (at files.pythonhosted.org) or the package metadata (at pypi.org)? It seems like only the latter is working atm. (But that's the more important one in any case).

@teonbrooks

This comment has been minimized.

Copy link
Member

teonbrooks commented May 2, 2019

what are your thoughts on micropip being bundled by default with pyodide in live in its namespace to be more like a public utility function like open_url? I think it might give the user a clear sign that it is being fetched and downloaded into the pyodide virtual fs and then could imported python like normally.

import pyodide
pyodide.micropip.install("package_name")
@mdboom

This comment has been minimized.

Copy link
Member Author

mdboom commented May 2, 2019

I don't have a problem with it being in pyodide, but it should be imported on-demand, since it's sorta-heavy (it requires distlib). But it's easy enough to make that import happen only when needed.

@mdboom mdboom force-pushed the mdboom:micropip branch from 4077277 to 3e3733e May 3, 2019
@mdboom mdboom merged commit c3c2f10 into iodide-project:master May 3, 2019
0 of 2 checks passed
0 of 2 checks passed
ci/circleci: build CircleCI is running your tests
Details
ci/circleci: test-python CircleCI is running your tests
Details
@SimonBiggs

This comment has been minimized.

Copy link

SimonBiggs commented May 3, 2019

This is awesome @mdboom. Thank you!

@JBBgameich

This comment has been minimized.

Copy link

JBBgameich commented Jul 27, 2019

It seems to me that the metadata from pypi can not be loaded using CORS anymore.
Firefox reports:
Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at https://pypi.org/pypi/youtube_dl/json. (Reason: CORS request did not succeed).

@SimonBiggs

This comment has been minimized.

Copy link

SimonBiggs commented Jul 27, 2019

I don't thing the cors issue ever did get fixed...

@filips123

This comment has been minimized.

Copy link
Contributor

filips123 commented Oct 6, 2019

@mdboom @SimonBiggs CORS support was already added to PyPI. See pypa/conveyor#5 (comment) for details. Can now this be added to use it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.