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

Fix compat with aiohttp 3.11.0+ #262

Merged
merged 1 commit into from
Nov 15, 2024
Merged

Conversation

bdraco
Copy link
Contributor

@bdraco bdraco commented Nov 13, 2024

This is still backwards compatible with older versions (tested with 3.10.11 and 3.9.5) as well.

Copy link

@cdessez cdessez left a comment

Choose a reason for hiding this comment

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

This looks sensible to me, although I admittedly don't know the internals of the library. Approving in spirit but I'm not sure this should be enough to merge it.

potiuk added a commit to potiuk/airflow that referenced this pull request Nov 13, 2024
Because of aio-libs/aiohttp#9866 some tests are failing. This PR can be
reverted when pnuckowski/aioresponses#262 is merged and released
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 14, 2024
Because of aio-libs/aiohttp#9866 some tests are failing. This PR can be
reverted when pnuckowski/aioresponses#262 is merged and released
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 14, 2024
Because of aio-libs/aiohttp#9866 some tests are failing. This PR can be
reverted when pnuckowski/aioresponses#262 is merged and released
@potiuk
Copy link

potiuk commented Nov 14, 2024

Yep. Would be great to have a fix for it released. If there is a way we can speed it up?

potiuk added a commit to apache/airflow that referenced this pull request Nov 14, 2024
Because of aio-libs/aiohttp#9866 some tests are failing. This PR can be
reverted when pnuckowski/aioresponses#262 is merged and released
amoghrajesh pushed a commit to astronomer/airflow that referenced this pull request Nov 14, 2024
Because of aio-libs/aiohttp#9866 some tests are failing. This PR can be
reverted when pnuckowski/aioresponses#262 is merged and released
@bdraco
Copy link
Contributor Author

bdraco commented Nov 14, 2024

We are going to release a 3.11.1 for backwards compat. https://github.com/aio-libs/aiohttp/actions/runs/11840386977

It would still be good to merge this change though since we don't expect RequestInfo objects to be created outside of aiohttp internals and it may break again in the future.

@bdraco
Copy link
Contributor Author

bdraco commented Nov 14, 2024

3.11.1 is available on PyPI

bdraco added a commit to home-assistant/core that referenced this pull request Nov 14, 2024
changelog: aio-libs/aiohttp@v3.11.0...v3.11.1

no functional changes, release was only to restore compat for
testing in pnuckowski/aioresponses#262
@potiuk
Copy link

potiuk commented Nov 14, 2024

3.11.1 is available on PyPI

Nice. Thanks!

@pnuckowski - is the aioresponses library maintained?

kaxil added a commit to astronomer/airflow that referenced this pull request Nov 14, 2024
kaxil added a commit to apache/airflow that referenced this pull request Nov 14, 2024
@pnuckowski pnuckowski merged commit 827d4c2 into pnuckowski:master Nov 15, 2024
20 checks passed
@cdessez
Copy link

cdessez commented Nov 15, 2024

Thanks, guys! 👍
Can you also trigger a release to have this fix available in the latest PyPI package?

@bdraco bdraco deleted the 3110_compat branch November 15, 2024 12:07
@pnuckowski
Copy link
Owner

Thanks, guys! 👍 Can you also trigger a release to have this fix available in the latest PyPI package?

today will upload, first need to fix some issues that occur while uploading to pypi :/ stay tuned

Alex-Izquierdo added a commit to ansible/ansible-rulebook that referenced this pull request Nov 15, 2024
Yesterday, 3.11 version of `aiohttp` was released with [breaking changes
](https://github.com/aio-libs/aiohttp/releases/tag/v3.11.0 ) which
breaks `aioresponses` library used for our mocks and as consequence some
of our tests are failing.
Ref: pnuckowski/aioresponses#262

Regardless of whether this is fixed in aioresponses, aiohttp can
introduce breaking changes in minor versions, so I pin it to the latest
tested version as this library is critical for the stability in some of
our features.

Signed-off-by: Alex <[email protected]>
@cdessez
Copy link

cdessez commented Nov 15, 2024

today will upload, first need to fix some issues that occur while uploading to pypi :/ stay tuned

Ok, thanks a lot, @pnuckowski. :) 🙏
It's also less pressing now since aiohttp published 3.13.1 to also fix the backwards compatibility issue on their side.

@potiuk
Copy link

potiuk commented Nov 15, 2024

Thanks @pnuckowski !

It's also less pressing now since aiohttp published 3.13.1 to also fix the backwards compatibility issue on their side.

But having it released might make them, to drop it back :)

@bdraco
Copy link
Contributor Author

bdraco commented Nov 15, 2024

Thanks @pnuckowski !

It's also less pressing now since aiohttp published 3.13.1 to also fix the backwards compatibility issue on their side.

But having it released might make them, to drop it back :)

Likely won't get dropped until 4.x (if ever) so no need to worry about that.

@pnuckowski
Copy link
Owner

released as 0.7.7

MoritzWeber0 added a commit to DSD-DBS/capella-collab-manager that referenced this pull request Nov 17, 2024
zkayyali812 pushed a commit to ansible/ansible-rulebook that referenced this pull request Dec 9, 2024
Yesterday, 3.11 version of `aiohttp` was released with [breaking changes
](https://github.com/aio-libs/aiohttp/releases/tag/v3.11.0 ) which
breaks `aioresponses` library used for our mocks and as consequence some
of our tests are failing.
Ref: pnuckowski/aioresponses#262

Regardless of whether this is fixed in aioresponses, aiohttp can
introduce breaking changes in minor versions, so I pin it to the latest
tested version as this library is critical for the stability in some of
our features.

Signed-off-by: Alex <[email protected]>
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