Skip to content

micropython/aiohttp: Add aiohttp package. #778

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

Merged
merged 1 commit into from
Dec 20, 2023

Conversation

Carglglz
Copy link
Contributor

@Carglglz Carglglz commented Dec 10, 2023

Implement an updated version of uaiohttpclient (renamed aiohttp) with ClientSession, WebSocketClient and SSLContext support, which is mostly compatible with CPython aiohttp-v3.9.1. client i.e. https://docs.aiohttp.org/en/stable/client_quickstart.html

This replaces #752 and #724 and requires:

@dpgeorge
Copy link
Member

This is a really great contribution, thank you!

A few top-level comments/questions:

  • Did you write this from scratch, or copy pieces from other locations (maybe uaiohttpclient)? The main question here is about licensing/copyright.
  • For the things that are implemented do they match CPython's aiohttp exactly? If not, can you document any differences?
  • This library should be placed in the python-ecosys directory, because it's aiming to match something that already exists (in the Python ecosystem). And the manifest.py should have the option pypi="aiohttp" added to the metadata() entry there (see other packages in the python-ecosys dir for hints).
  • I suggest to keep the old uaiohttpclient library as-is, and it can be deprecated and removed in the future. Essentially aiohttp will be a completely new package.

@Carglglz Carglglz force-pushed the aiohttp-websocket branch 2 times, most recently from 4999119 to 2f8238f Compare December 19, 2023 04:31
@Carglglz
Copy link
Contributor Author

Thanks for the feedback!

Did you write this from scratch, or copy pieces from other locations (maybe uaiohttpclient)? The main question here is about licensing/copyright.

The original idea was to add this to uaiohttpclient , but I felt that it was a bit outdated so I though it would be better to add this as a replacement. ( although I agree with keeping uaiohttpclient as-is). So the source code I used comes from:

For the things that are implemented do they match CPython's aiohttp exactly? If not, can you document any differences?

I would say so, e.g. all examples run as expected in MicroPython and CPython.

The TLDR of the things that are implemented is:
most features of https://docs.aiohttp.org/en/stable/client_quickstart.html (except Timeouts, and file uploads)
and some of https://docs.aiohttp.org/en/stable/client_advanced.html (i.e. headers and SSLContext)

I think the main difference is that MicroPython implementation is missing some keyword arguments in ClientSession and in requests method (I may update this with the "exact" diff if necessary)

Also if you see anything that can be improved let me know or feel free to add any changes and I can review them later (whatever is faster 👍🏼 )

)

def get(self, url, ssl=None, params=None, headers={}):
return _RequestContextManager(
Copy link
Member

Choose a reason for hiding this comment

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

To simplify and reduce code, all these convenience functions could be written like this:

def get(self, url, **kwargs):
    return self.request("GET", url, **kwargs)

Even if that's slightly different in terms of allowed args (eg get now allows the data argument) I think it's worth doing because it greatly simplifies these functions.

@dpgeorge
Copy link
Member

So the source code I used comes from:
...

Great, thanks for those details. That all looks fine, the code is all MIT.

The TLDR of the things that are implemented is: most features of https://docs.aiohttp.org/en/stable/client_quickstart.html (except Timeouts, and file uploads) and some of https://docs.aiohttp.org/en/stable/client_advanced.html (i.e. headers and SSLContext)

That's really good, getting the quick-start code mostly working is great.

I think the main difference is that MicroPython implementation is missing some keyword arguments in ClientSession and in requests method (I may update this with the "exact" diff if necessary)

That's fine. These things can be added in the future if needed.

Implement `aiohttp` with `ClientSession`, websockets and `SSLContext`
support.

Only client is implemented and API is mostly compatible with CPython
`aiohttp`.

Signed-off-by: Carlos Gil <carlosgilglez@gmail.com>
@dpgeorge dpgeorge merged commit 7cdf708 into micropython:master Dec 20, 2023
@dpgeorge
Copy link
Member

Thanks for updating. Now merged.

@Carglglz Carglglz deleted the aiohttp-websocket branch December 21, 2023 16:41
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