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

potential aiohttp>=4.0.0 breaking changes #882

Closed
terricain opened this issue Aug 30, 2021 · 7 comments · Fixed by #1081
Closed

potential aiohttp>=4.0.0 breaking changes #882

terricain opened this issue Aug 30, 2021 · 7 comments · Fixed by #1081
Assignees
Labels

Comments

@terricain
Copy link
Collaborator

Describe the bug
I have a suspicion we'll hit some issues when aiohttp 4.0.0 comes out judging by what I've seen in this issue terricain/aioboto3#237 where the user had installed 4.0.0a1.

The issue was using aiobotocore 1.3.3, where verify_ssl was an unexpected argument passed to aiohttp.TCPConnector, aiohttp.TCPConnector has since moved here but I reckon it would still have the same issue.

I shall look into this in more depth later today. Might be worth updating the dep to aiohttp>=3.3.1,<4.0.0.

@jon-rosenberg-rse
Copy link

Running into this issue today pulling in aiobotocore 2.2.0 as a dependency to s3fs. Your pipfile specified
aiohttp = "==3.7.4.post0"
Will still work, but the setup.py aiohttp>=3.3.1 allows 4.0.0 to be installed which does indeed have your referenced verify_ssl issue.

@thehesiod
Copy link
Collaborator

cool will work on this asap, probably we need to test with both versions

@leandrorebelo
Copy link

Some news about this issue?

@brianmedigate
Copy link

It looks like this just broke with aiohttp 3.9.2, which includes this change: aio-libs/aiohttp#8043

@terricain
Copy link
Collaborator Author

That shouldn't be too bad a fix, I reckon https://github.com/aio-libs/aiobotocore/blob/master/aiobotocore/httpsession.py#L114 removing verify_ssl argument and changing ssl to ssl=ssl_context if ssl_context is not None else bool(verify) should suffice and satisfy the arg checking logic here https://github.com/aio-libs/aiohttp/blob/v3.9.2/aiohttp/client_reqrep.py#L156

@jakob-keller
Copy link
Collaborator

I'll look into it. There's #1077 fixing HTTPS proxy support that looks related to this issue.

@jakob-keller jakob-keller self-assigned this Jan 29, 2024
@Dreamsorcerer
Copy link
Member

That shouldn't be too bad a fix, I reckon https://github.com/aio-libs/aiobotocore/blob/master/aiobotocore/httpsession.py#L114 removing verify_ssl argument and changing ssl to ssl=ssl_context if ssl_context is not None else bool(verify) should suffice and satisfy the arg checking logic here https://github.com/aio-libs/aiohttp/blob/v3.9.2/aiohttp/client_reqrep.py#L156

Just set the start value of ssl_context to bool(verify) instead of None. That should then be compatible with 3.9.2+ including 4+. We'll fix this this backwards-compatible breakage in 3.9.3 though, that certainly wasn't intended. We convert None in ClientSession, but missed the case where you're sending it to the connector directly.

jakob-keller added a commit to jakob-keller/aiobotocore that referenced this issue Jan 29, 2024
jakob-keller added a commit to jakob-keller/aiobotocore that referenced this issue Jan 29, 2024
jakob-keller added a commit to jakob-keller/aiobotocore that referenced this issue Jan 29, 2024
jakob-keller added a commit to jakob-keller/aiobotocore that referenced this issue Jan 29, 2024
jakob-keller added a commit to jakob-keller/aiobotocore that referenced this issue Jan 30, 2024
jakob-keller added a commit to jakob-keller/aiobotocore that referenced this issue Feb 3, 2024
jakob-keller added a commit to jakob-keller/aiobotocore that referenced this issue Mar 4, 2024
jakob-keller added a commit to jakob-keller/aiobotocore that referenced this issue Apr 11, 2024
jakob-keller added a commit to jakob-keller/aiobotocore that referenced this issue May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants