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

Emscripten support #3330

Open
wants to merge 53 commits into
base: master
Choose a base branch
from
Open

Emscripten support #3330

wants to merge 53 commits into from

Conversation

joemarshall
Copy link
Contributor

@joemarshall joemarshall commented Oct 2, 2024

Summary

I added a discussion for this ages back but there's been no input, so I've written it (because I was contracted to do the work anyway, so I might as well contribute it upstream). This PR adds support for running in emscripten / webassembly platforms, where all network connections go via the browser.

Currently in progress, but tests okay locally, so I've opened this to check the CI changes, I've got to update docs also.

Checklist

  • [X ] I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.

@tomchristie
Copy link
Member

This is really interesting, thanks... ☺️

I've taken a bit of a look at the ecosystem here, tho am going to need a bit more orientation... Would it make sense to document an example of how to write an HTML page that includes a Python REPL with httpx imported and available?

@joemarshall
Copy link
Contributor Author

@tomchristie I put in the PR that makes import ssl optional now (#3385 )

I updated this PR so it follows on from that PR.

How this PR works now is it moves _transports.default into _transports.httpcore, which defines [Async]HTTPCoreTransport, adds an extra module _transports.jsfetch file which defines [Async]JavascriptFetchTransport. Then in _transports/__init__.py it adds an alias of HTTPTransport which goes to whichever HTTP backend is in use (i.e. httpcore by default, JS fetch on emscripten)

@joemarshall
Copy link
Contributor Author

@tomchristie I updated this to follow the changes in master - I think #3385 is redundant now, as the ssl changes are minimal at this point.

@joemarshall joemarshall marked this pull request as ready for review November 13, 2024 10:29
httpx/_models.py Outdated Show resolved Hide resolved
@samuelcolvin
Copy link

I'll add that I've been playing with httpx on https://pydantic.run over the last few days, both sync and async, and apart from the prints reported in pyodide/pyodide#5381, it seems to be working well otherwise!

joemarshall and others added 2 commits January 29, 2025 12:59
Co-authored-by: Samuel Colvin <[email protected]>
Co-authored-by: Samuel Colvin <[email protected]>
@joemarshall
Copy link
Contributor Author

Oh darn, can't believe I missed those debug prints. Fixed now.

@samuelcolvin
Copy link

Thanks so much @joemarshall for fixing those.

@hoodmane or @joemarshall, I'm not sure what the process is (or I'd try to help), but please can we update pyodide to use the head of his PR to avoid those debug print statements confusing users.

@tomchristie anything stopping this being merged?

@hoodmane
Copy link

hoodmane commented Feb 8, 2025

I think Joe Marshall already sent a pr to Pyodide to update it so when we make another release it will bring it in.

@samuelcolvin
Copy link

I'm getting this error when using openai in pyodide, looks like the openai SDK is assuming something is bytes when it's actually a memoryview

  File "/lib/python3.12/site-packages/openai/_streaming.py", line 147, in __aiter__
    async for item in self._iterator:
  File "/lib/python3.12/site-packages/openai/_streaming.py", line 160, in __stream__
    async for sse in iterator:
  File "/lib/python3.12/site-packages/openai/_streaming.py", line 151, in _iter_events
    async for sse in self._decoder.aiter_bytes(self.response.aiter_bytes()):
  File "/lib/python3.12/site-packages/openai/_streaming.py", line 302, in aiter_bytes
    async for chunk in self._aiter_chunks(iterator):
  File "/lib/python3.12/site-packages/openai/_streaming.py", line 314, in _aiter_chunks
    for line in chunk.splitlines(keepends=True):
                ^^^^^^^^^^^^^^^^
AttributeError: 'memoryview' object has no attribute 'splitlines'

Any chance the error is related to this PR?

it works fine locally, and it works fine when not using their streaming responses. Happy to give more details or create a separate issue if that helps?

@tomchristie
Copy link
Member

anything stopping this being merged?

I would suggest not making any PR/changes to httpx, and start by demo'ing an Emscripten transport.

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.

5 participants