Leverage aiohttp internal handling of `chunked` argument

[jakob-keller]

Description of Change

https://github.com/aio-libs/aiobotocore/pull/1077#pullrequestreview-1912935492

Assumptions

Replace this text with any assumptions made (if any)

Checklist for All Submissions

• [x] 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):
  • [ ] 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

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 85.94%. Comparing base (aec730a) to head (cdd466e).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1096      +/-   ##
==========================================
- Coverage   86.11%   85.94%   -0.17%     
==========================================
  Files          62       62              
  Lines        5933     5928       -5     
==========================================
- Hits         5109     5095      -14     
- Misses        824      833       +9     

Flag	Coverage Δ
unittests	85.94% <100.00%> (-0.17%)	:arrow_down:

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[thehesiod]

is this change covered by tests? AFK for a bit to check the coverage

[jakob-keller]

is this change covered by tests? AFK for a bit to check the coverage

I am not sure and we touched upon that here: https://github.com/aio-libs/aiobotocore/pull/1077#pullrequestreview-1912935492.

For our purposes, aiobotocore's chunked() duplicates logic already provided by aiohttp's update_transfer_encoding(): https://github.com/aio-libs/aiohttp/blob/7571eb9f65fde72fd6ccadcc5bfe2dc81f3a126c/aiohttp/client_reqrep.py#L439

I believe it is safe to drop chunked() when modern releases of aiohttp are used, i.e. any release we support in requirements.

[jakob-keller]

@Dreamsorcerer: Could you advise us on this PR, please? I believe that aiohttp will handle chunked requests correctly, without having to pass the chunked argument explicitely. Any guidance?

[Dreamsorcerer]

From reading the code, it looks like in the code, if the body has no size and no Content-Length header is set, then chunked will be set regardless of what you pass in: https://github.com/aio-libs/aiohttp/blob/3de518a0a324b082cca3ed94ef5a9338b4745759/aiohttp/client_reqrep.py#L503

Then it looks like it will handle chunked as per: https://github.com/aio-libs/aiohttp/blob/3de518a0a324b082cca3ed94ef5a9338b4745759/aiohttp/client_reqrep.py#L444-L457

From my reading of that, the code expects to set the Transfer-Encoding header itself. Which is probably why the code you're deleting removed the Transfer-Encoding header. So, I'd suggest spending a bit more time understanding why a Transfer-Encoding header would have been set in first place, seems to me that there should never be a Transfer-Encoding header passed in here.

[jakob-keller]

From my reading of that, the code expects to set the Transfer-Encoding header itself. Which is probably why the code you're deleting removed the Transfer-Encoding header. So, I'd suggest spending a bit more time understanding why a Transfer-Encoding header would have been set in first place, seems to me that there should never be a Transfer-Encoding header passed in here.

Thank you, that helped a lot!

Transfer-Encoding headers are set by botocore: https://github.com/boto/botocore/blob/149ae67cc8909ab03c11337392e676711a438d90/botocore/awsrequest.py#L405

We are removing them to satisfy expectations of aiohttp.

Everything works as it should.