Skip to content

Address breaking change introduced in aiohttp==3.9.2#1081

Merged
thehesiod merged 3 commits intoaio-libs:masterfrom
jakob-keller:aiohttp-ssl
May 16, 2024
Merged

Address breaking change introduced in aiohttp==3.9.2#1081
thehesiod merged 3 commits intoaio-libs:masterfrom
jakob-keller:aiohttp-ssl

Conversation

@jakob-keller
Copy link
Collaborator

@jakob-keller jakob-keller commented Jan 29, 2024 •
edited
Loading

Description of Change

aiohttp==3.9.2 introduces a change in handling SSL/TLS-related arguments that breaks our code. This PR applies current aiohttp best-practices to SSL/TLS-related arguments.

Assumptions

  • The PR is based on Fix use of HTTPS proxies #1070 #1077 which fixes HTTPS proxy support and is thus related.
  • The dependency specification for aiohttp was bumped to prevent regressions with our updated code and aiohttp<3.9.2.

Checklist for All Submissions

  • I have added change info to CHANGES.rst
  • If this is resolving an issue (needed so future developers can determine if change is still necessary and under what conditions) (can be provided via link to issue with these details): closes potential aiohttp>=4.0.0 breaking changes #882
    • Detailed description of issue
    • Alternative methods considered (if any)
    • How issue is being resolved
    • How issue can be reproduced
  • If this is providing a new feature (can be provided via link to issue with these details):
    • Detailed description of new feature
    • Why needed
    • Alternatives methods considered (if any)

Checklist when updating botocore and/or aiohttp versions

  • I have read and followed CONTRIBUTING.rst
  • I have updated test_patches.py where/if appropriate (also check if no changes necessary)
  • I have ensured that the awscli/boto3 versions match the updated botocore version

@jakob-keller jakob-keller self-assigned this Jan 29, 2024
@jakob-keller jakob-keller added bug Something isn't working deps-update labels Jan 29, 2024
@jakob-keller jakob-keller force-pushed the aiohttp-ssl branch 3 times, most recently from 3b7b25c to 99b9244 Compare January 29, 2024 17:53
@jakob-keller jakob-keller marked this pull request as ready for review January 29, 2024 17:57
@codecov
Copy link

codecov bot commented Jan 30, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.28%. Comparing base (2d6520b) to head (05c9321).
Report is 71 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #1081      +/-   ##
==========================================
- Coverage   86.33%   86.28%   -0.06%     
==========================================
  Files          62       62              
  Lines        5913     5913              
==========================================
- Hits         5105     5102       -3     
- Misses        808      811       +3
Flag Coverage Δ
unittests 86.28% <100.00%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dacevedo12
Copy link

dacevedo12 commented Jan 30, 2024

Seems like they addressed this in 3.9.3 aio-libs/aiohttp#8098

@jakob-keller
Copy link
Collaborator Author

jakob-keller commented Jan 30, 2024

Seems like they addressed this in 3.9.3 aio-libs/aiohttp#8098

Indeed, still we should cease using the deprecated verify_ssl argument to prevent this issue from popping up again in the future.

@jakkdl
Copy link
Contributor

jakkdl commented Feb 1, 2024

Seems like they addressed this in 3.9.3 aio-libs/aiohttp#8098

Indeed, still we should cease using the deprecated verify_ssl argument to prevent this issue from popping up again in the future.

perhaps of note is that #749 (which I've been slowly chugging away on) wants to replace aiohttp with httpx, so there might not be a very long future for aiohttp.

@jakob-keller
Copy link
Collaborator Author

jakob-keller commented Feb 3, 2024

@thehesiod: This PR builds on #1077. Maybe we should first finish that PR?

thehesiod
thehesiod reviewed Feb 3, 2024
View reviewed changes
@jakob-keller jakob-keller marked this pull request as draft February 28, 2024 21:10
@jakob-keller jakob-keller marked this pull request as ready for review March 4, 2024 19:48
@jakob-keller jakob-keller requested a review from thehesiod March 4, 2024 19:48
thehesiod
thehesiod requested changes Mar 4, 2024
View reviewed changes
@jakob-keller jakob-keller requested a review from thehesiod May 16, 2024 18:47
@thehesiod thehesiod enabled auto-merge (squash) May 16, 2024 18:55
@jakob-keller
Copy link
Collaborator Author

jakob-keller commented May 16, 2024

CI doesn't allow for two releases dated the same in CHANGES.txt :-(

@thehesiod
Copy link
Collaborator

thehesiod commented May 16, 2024

will fix test here, one sec

@thehesiod thehesiod merged commit 752f8b5 into aio-libs:master May 16, 2024
@jakob-keller jakob-keller deleted the aiohttp-ssl branch May 16, 2024 19:15
@jakob-keller jakob-keller mentioned this pull request Aug 17, 2024
Merged
13 tasks
@jakob-keller jakob-keller added dependencies Pull requests that update a dependency file and removed deps-update labels Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thehesiod thehesiod thehesiod approved these changes

@terricain terricain Awaiting requested review from terricain

Assignees

@jakob-keller jakob-keller

Labels

bug Something isn't working dependencies Pull requests that update a dependency file

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

potential aiohttp>=4.0.0 breaking changes

4 participants

@jakob-keller @dacevedo12 @jakkdl @thehesiod