Add client Brotli support #5227

derlih · 2020-11-13T16:55:11Z

What do these changes do?

If brotlipy is installed, client will support this compression type

Are there changes in behavior for the user?

No

Related issue number

#5219

Checklist

I think the code is well written
Unit tests for the changes exist
Documentation reflects the changes
If you provide code modification, please add yourself to CONTRIBUTORS.txt
- The format is <Name> <Surname>.
- Please keep alphabetical order, the file is sorted by names.
Add a new news fragment into the CHANGES folder
- name it <issue_id>.<type> for example (588.bugfix)
- if you don't have an issue_id change it to the pr id after creating the pr
- ensure type is one of the following:
  - .feature: Signifying a new feature.
  - .bugfix: Signifying a bug fix.
  - .doc: Signifying a documentation improvement.
  - .removal: Signifying a deprecation or removal of public API.
  - .misc: A ticket has been closed, but it is not of interest to users.
- Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."

CLAassistant · 2020-11-13T16:55:19Z

All committers have signed the CLA.

codecov · 2020-11-13T17:00:42Z

Codecov Report

Merging #5227 (58140ae) into master (a2fef0f) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #5227   +/-   ##
=======================================
  Coverage   97.54%   97.54%           
=======================================
  Files          43       43           
  Lines        8794     8797    +3     
  Branches     1413     1413           
=======================================
+ Hits         8578     8581    +3     
  Misses        101      101           
  Partials      115      115

Flag	Coverage Δ
unit	`97.39% <100.00%> (+<0.01%)`	⬆️

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

Impacted Files	Coverage Δ
aiohttp/client_reqrep.py	`97.39% <100.00%> (+0.01%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a2fef0f...58140ae. Read the comment docs.

asvetlov

The PR looks good.

Please click on https://cla-assistant.io/aio-libs/aiohttp?pullRequest=5227 to sign CLA.
I've added it recently and checked -- the sign is pretty easy :)

derlih · 2020-11-13T23:35:53Z

@asvetlov
Is aiohttp somehow connected to SAP?

asvetlov · 2020-11-14T11:58:23Z

Oops.
No, sure! Thanks for the note.
I've updated the CLA text to closely follow the Python CLA (https://www.python.org/psf/contrib/contrib-form/) but slightly adapted to the aio-libs project.
The CLA text is available here: https://gist.github.com/asvetlov/dbce6679052f5c89d3d13f501107c1d4

derlih · 2020-11-14T12:35:28Z

Signed.
Should the brotli runtime support be reflected in somewhere in docs?

asvetlov · 2020-11-14T12:43:57Z

Should the brotli runtime support be reflected in somewhere in docs?

Sounds good.
Do you have an idea of what place the sentence should be pushed in?

derlih · 2020-11-14T12:58:48Z

What about the dependencies section?

aiohttp/docs/index.rst

Lines 146 to 170 in 021668a

    
           Dependencies 
        
           ============ 
        
           - Python 3.6+ 
        
           - *async_timeout* 
        
           - *attrs* 
        
           - *chardet* 
        
           - *multidict* 
        
           - *yarl* 
        
           - *Optional* :term:`cchardet` as faster replacement for 
        
             :term:`chardet`. 
        
             Install it explicitly via: 
        
             .. code-block:: bash 
        
                $ pip install cchardet 
        
           - *Optional* :term:`aiodns` for fast DNS resolving. The 
        
             library is highly recommended. 
        
             .. code-block:: bash 
        
                $ pip install aiodns

asvetlov · 2020-11-14T13:07:58Z

ok

docs/glossary.rst

asvetlov

Ooops, the spell checker doesn't approve the docs changes: https://github.com/aio-libs/aiohttp/pull/5227/checks?check_run_id=1400046288

The most practical solution is extending https://github.com/aio-libs/aiohttp/blob/master/docs/spelling_wordlist.txt with words like brotli to teach the checker new facilities

derlih · 2020-11-15T19:30:27Z

I fixed the spellcheck.

asvetlov · 2020-11-15T21:28:09Z

The feature is backward compatible.
But it is still a new feature, there is no need to backport it to 3.7

derlih · 2020-11-16T07:27:23Z

The aiohttp user from #5219 wanted to use it. May be for 3.7.3?
I think this release will happen quite soon because the win32 wheels are currently missing.

asvetlov · 2020-11-18T19:50:32Z

Sorry, I had no capacity to push it into 3.7.3.
Technically this is a new backward-compatible feature.
It means that it can be released as a part of 3.8.0 pretty soon.
I want to review recent reproducible bug reports (and fix them if possible) first to include fixes into 3.8

asvetlov · 2020-11-23T08:21:55Z

Thanks!

github-actions · 2020-11-23T08:22:06Z

💔 Backport was not successful

The PR was attempted backported to the following branches:

❌ 3.8: Commit could not be cherrypicked due to conflicts

ddelange · 2022-11-24T12:17:11Z

hi 👋

it seems this patch didn't make it into 3.8.3, the Accept-Encoding header is still hard-coded.

could this patch be cherry-picked into 3.8.4?

patchback · 2022-11-24T13:54:32Z

Backport to 3.9: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply 28ea32d on top of patchback/backports/3.9/28ea32d2282728a94af73c87efd6ab314c14320e/pr-5227

Backporting merged PR #5227 into master

Ensure you have a local repo clone of your fork. Unless you cloned it
from the upstream, this would be your origin remote.
Make sure you have an upstream repo added as a remote too. In these
instructions you'll refer to it by the name upstream. If you don't
have it, here's how you can add it:
```
$ git remote add upstream https://github.com/aio-libs/aiohttp.git
```

Ensure you have the latest copy of upstream and prepare a branch
that will hold the backported code:

$ git fetch upstream
$ git checkout -b patchback/backports/3.9/28ea32d2282728a94af73c87efd6ab314c14320e/pr-5227 upstream/3.9

Now, cherry-pick PR Add client Brotli support #5227 contents into that branch:
```
$ git cherry-pick -x 28ea32d2282728a94af73c87efd6ab314c14320e
```
If it'll yell at you with something like fatal: Commit 28ea32d2282728a94af73c87efd6ab314c14320e is a merge but no -m option was given., add -m 1 as follows intead:
```
$ git cherry-pick -m1 -x 28ea32d2282728a94af73c87efd6ab314c14320e
```
At this point, you'll probably encounter some merge conflicts. You must
resolve them in to preserve the patch from PR Add client Brotli support #5227 as close to the
original as possible.

Push this branch to your fork on GitHub:

$ git push origin patchback/backports/3.9/28ea32d2282728a94af73c87efd6ab314c14320e/pr-5227

Create a PR, ensure that the CI is green. If it's not — update it so that
the tests and any other checks pass. This is it!
Now relax and wait for the maintainers to process your pull request
when they have some cycles to do reviews. Don't worry — they'll tell you if
any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

webknjaz · 2022-11-24T13:54:55Z

You can try following bot instructions to submit a PR against 3.9.

(cherry picked from commit 28ea32d)

(cherry picked from commit 28ea32d) (cherry picked from commit fab2c62)

ddelange · 2022-11-24T17:52:46Z

opened backport PRs against 3.9 (#7100) and 3.8 (#7101)

(cherry picked from commit 28ea32d) backport ref #5227 (comment) Co-authored-by: Dmitry Erlikh <derlih@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

ddelange · 2023-04-19T06:44:25Z

hi 👋 is there a rough ETA when 3.9 and/or 4.0 will be shipped?

derlih requested review from asvetlov and webknjaz as code owners November 13, 2020 16:55

psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Nov 13, 2020

asvetlov requested changes Nov 13, 2020

View reviewed changes

derlih added 3 commits November 14, 2020 14:42

Add client Brotli support

a48140f

Add changes file

6a0a173

Update docs

bc6f958

derlih force-pushed the brotli branch from 155a7e1 to bc6f958 Compare November 14, 2020 13:42

asvetlov requested changes Nov 14, 2020

View reviewed changes

docs/glossary.rst Outdated Show resolved Hide resolved

docs: brotlipy -> Brotli

41bebec

derlih changed the title ~~Add client Brotli support~~ [WIP] Add client Brotli support Nov 14, 2020

requirement/test.txt brotlipy -> Brotli

a58b7ed

derlih changed the title ~~[WIP] Add client Brotli support~~ Add client Brotli support Nov 14, 2020

fix test

040458d

asvetlov approved these changes Nov 14, 2020

View reviewed changes

asvetlov requested changes Nov 14, 2020

View reviewed changes

derlih added 2 commits November 14, 2020 16:50

Spellcheck

7dfc581

fixup! Spellcheck

58140ae

asvetlov approved these changes Nov 15, 2020

View reviewed changes

asvetlov added the backport-3.8 label Nov 15, 2020

asvetlov merged commit 28ea32d into aio-libs:master Nov 23, 2020

derlih deleted the brotli branch November 23, 2020 08:40

commonism pushed a commit to commonism/aiohttp that referenced this pull request Apr 27, 2021

Add client Brotli support (aio-libs#5227)

fd3174f

commonism pushed a commit to commonism/aiohttp that referenced this pull request Apr 27, 2021

Add client Brotli support (aio-libs#5227)

b9ceabd

webknjaz added backport-3.9 Trigger automatic backporting to the 3.9 release branch by Patchback robot and removed backport-3.8 labels Nov 24, 2022

ddelange pushed a commit to ddelange/aiohttp that referenced this pull request Nov 24, 2022

Add client Brotli support (aio-libs#5227)

fab2c62

(cherry picked from commit 28ea32d)

ddelange mentioned this pull request Nov 24, 2022

Add client Brotli support (#5227) #7100

Merged

ddelange pushed a commit to ddelange/aiohttp that referenced this pull request Nov 24, 2022

Add client Brotli support (aio-libs#5227)

69f6f18

(cherry picked from commit 28ea32d) (cherry picked from commit fab2c62)

ddelange mentioned this pull request Nov 24, 2022

Add client Brotli support (#5227) #7101

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add client Brotli support #5227

Add client Brotli support #5227

derlih commented Nov 13, 2020 •

edited

CLAassistant commented Nov 13, 2020 •

edited

codecov bot commented Nov 13, 2020 •

edited

asvetlov left a comment

derlih commented Nov 13, 2020

asvetlov commented Nov 14, 2020

derlih commented Nov 14, 2020

asvetlov commented Nov 14, 2020

derlih commented Nov 14, 2020

asvetlov commented Nov 14, 2020

asvetlov left a comment

derlih commented Nov 15, 2020

asvetlov commented Nov 15, 2020 •

edited

derlih commented Nov 16, 2020

asvetlov commented Nov 18, 2020

asvetlov commented Nov 23, 2020

github-actions bot commented Nov 23, 2020

ddelange commented Nov 24, 2022 •

edited

patchback bot commented Nov 24, 2022 •

edited

webknjaz commented Nov 24, 2022

ddelange commented Nov 24, 2022

ddelange commented Apr 19, 2023

Add client Brotli support #5227

Add client Brotli support #5227

Conversation

derlih commented Nov 13, 2020 • edited

What do these changes do?

Are there changes in behavior for the user?

Related issue number

Checklist

CLAassistant commented Nov 13, 2020 • edited

codecov bot commented Nov 13, 2020 • edited

Codecov Report

asvetlov left a comment

Choose a reason for hiding this comment

derlih commented Nov 13, 2020

asvetlov commented Nov 14, 2020

derlih commented Nov 14, 2020

asvetlov commented Nov 14, 2020

derlih commented Nov 14, 2020

asvetlov commented Nov 14, 2020

asvetlov left a comment

Choose a reason for hiding this comment

derlih commented Nov 15, 2020

asvetlov commented Nov 15, 2020 • edited

derlih commented Nov 16, 2020

asvetlov commented Nov 18, 2020

asvetlov commented Nov 23, 2020

github-actions bot commented Nov 23, 2020

💔 Backport was not successful

ddelange commented Nov 24, 2022 • edited

patchback bot commented Nov 24, 2022 • edited

Backport to 3.9: 💔 cherry-picking failed — conflicts found

Backporting merged PR #5227 into master

webknjaz commented Nov 24, 2022

ddelange commented Nov 24, 2022

ddelange commented Apr 19, 2023

derlih commented Nov 13, 2020 •

edited

CLAassistant commented Nov 13, 2020 •

edited

codecov bot commented Nov 13, 2020 •

edited

asvetlov commented Nov 15, 2020 •

edited

ddelange commented Nov 24, 2022 •

edited

patchback bot commented Nov 24, 2022 •

edited