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

merge with https://github.com/emscripten-forge/requests-wasm-polyfill #6

Open
DerThorsten opened this issue Sep 19, 2022 · 5 comments
Assignees

Comments

@DerThorsten
Copy link

DerThorsten commented Sep 19, 2022

Hi, I am the author of https://github.com/emscripten-forge/requests-wasm-polyfill where we do essentially the same but for emscripten-forge instead of pyodide.
Maybe we can merge these two libraries into a single one.

Greetings Thorsten

@koenvo
Copy link
Owner

koenvo commented Sep 20, 2022

Thanks for reaching out! This sounds like a great idea. I believe I need to read a bit more about emscripten-forge. Are there any good articles about it? Also about how it relates to pyodide?

It seems requests-wasm-polyfill should work like a replacement for requests, right? Does it also require pyodide/micropip#9 when dependencies require requests?

For the pyodide-http package the goal is to patch common used http packages. By patching it allows normal installation of packages including their dependencies.

I was thinking about multiple packages:

  1. Core for patching existing libraries
  2. replacement package for requests (like requests-wasm-polyfill; which can be installed using micropip redirect) that installs the Core and does a patch_requests

Curious to your vision on merging both packages!

@koenvo
Copy link
Owner

koenvo commented Sep 30, 2022

@DerThorsten

@emirkmo
Copy link

emirkmo commented Oct 18, 2022

Hi @koenvo (and @DerThorsten),

I've had a feature request to add requests API wrapper to pyodide (or just downstream to pyscript, as that's where I would like to use it). The idea would be to provide a familiar API for python devs. Although I would prefer it to be async or at least optionally async, but pyodide dev seemed to think it should not be async (follow requests API 1 to 1).

See issue: pyodide/pyodide#3160

My idea was to build a wrapper around pyodide.pyfetch (or just fetch api) to correspond to the BASIC requests API. Step 1 would be just to have the familiar API from: https://github.com/psf/requests/blob/main/requests/api.py and the return types etc. can be fixed to follow requests in the future (currently they would just be the FetchResponse from pyfetch, or similar). Streaming requests and other bells and whistles are great in the future as well.

The idea was inspired by me writing this how-to guide for pyscript: https://docs.pyscript.net/latest/howtos/http-requests.html
where I wrote a convenience function called request just to provide familiarity to newcomers. Also by multiple issues on the pyscript repo.

I am wondering how close pyodide-http is towards this goal? I am also a bit confused I guess because you aim to always patch the requests import, correct? How easy would it be to not have the patching, but simply provide an implementation of the same API? I guess similar to the "replacement package for requests" that you mention above. Are there pitfalls to using it inside pyscript?

The usecase I have in mind is:

from pyodide-http import requests

requests.get(...)

Although the name should be changed :P to avoid confusion with importing actual pyodide-http imo. :P

Finally, two questions:

  • Are there issues with wrapping the underlying async code in non-async functions? (Is a custom stack needed? Are there pitfalls I am not thinking of?)

  • Finally, how close is this to being added to pyodide for easy import inside pyscript? Or is it already useable? By this I mean that it is either shipped with pyodide or can be installed from https://github.com/pyodide/pyodide/tree/main/packages or via micropip. Ideally providing the possibility of doing a simple import of requests (without needing to apply the patch). Basically, how easily could you pyscript provide this as a requests workaround?

@koenvo
Copy link
Owner

koenvo commented Nov 26, 2022

@emirkmo i totally missed your comment here. Sorry about that. I’m still a bit busy but will respond asap.

@koenvo koenvo self-assigned this Nov 26, 2022
@koenvo
Copy link
Owner

koenvo commented Nov 27, 2022

Hi @emirkmo,

Thanks for your message.

I agree on the need to change the name of this package. Let me create a issue for that to make sure I don't forget what needs to be done when renaming a package. Some larges projects, like Panel, are already using this package and don't want to break something there.

You blog looks very interesting! In a browser context it makes a lot of sense to use async code for network requests instead of blocking the main thread. This does also require people to write all their code async as there is no way to make async calls synchronous. When people do network requests in their own code they should use async calls.

A lot of packages do synchronous calls and therefore require a patch to make them work in pyodide environments. The main purpose if this package - right now - is to make those packages work without having the end-user to do anything expect call a pyodide_http.patch_all().

I do think it makes a lot of sense to have a single package that can do the patching and provide a replacement for requests. But you have to keep in mind the browser context gives a load of restrictions and limited api's, like @rth mentioned in pyodide/pyodide#3160 (comment) . With development of the browser api's more and more functionality can be added.

Maybe a wild idea, but an option could be to include a replacement for requests in pyodide which only informs the end-user they should use pyodide-http to make it work. That way the end-user knows requests will not fully work as expected. Curious what @rth thinks about this.

And finally: this package is already used in pyscript. See #20 (comment)

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

3 participants