Add server capability to check for Brotli compressed static files by steverep · Pull Request #8063 · aio-libs/aiohttp

steverep · 2024-01-26T20:26:08Z

What do these changes do?

Currently server only checks if static routes have a .gz extension and serves them with gzip encoding. These changes do the same for .br files with br encoding. Brotli is prioritized over gzip if both exist and are supported by the client, as it should almost always be a smaller content length.

I considered making a check for which is smaller if both exist, but figured it wouldn't be worth the extra file system call in the vast majority of cases (at least not for typical web formats). Users should simply use gzip if it's smaller than Brotli for any file.

Are there changes in behavior for the user?

Only change would be if users are already including .br files, then obviously those would start being served, but IMO that's not "breaking" since the content should be the same.

Related issue number

Fixes #8062

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."

aiohttp/web_response.py

aiohttp/web_fileresponse.py

tests/test_web_sendfile_functional.py

bdraco · 2024-01-26T21:16:28Z

I think this one should be safe for backport.

bdraco

Thanks for the PR. Please see comments above

aiohttp/web_fileresponse.py

aiohttp/web_response.py

Dreamsorcerer · 2024-01-26T22:44:59Z

@bdraco I'd suggest this should only go to 3.10 backport. We try to stick to bug fixes in the patch releases.

aiohttp/web_fileresponse.py

This reverts commit b0d58f7.

Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>

aiohttp/web_fileresponse.py

tests/hello.txt.br

tests/test_web_sendfile_functional.py

webknjaz

Blockers so far:

new binary blob in Git
formatting changes in tests

Please, deal with this to making this mergeable.

This reverts commit 634bf94.

Looks like all of my comments have been resolved so that leaves #8063 (review)

aiohttp/web_fileresponse.py

bdraco

LGTM, #8136 should merge first so it can be backported

patchback · 2024-02-14T12:34:28Z

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

❌ Failed to cleanly apply dfc9296 on top of patchback/backports/3.10/dfc92967d10eb83a8d726c02c3de90da15f8335f/pr-8063

Backporting merged PR #8063 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.10/dfc92967d10eb83a8d726c02c3de90da15f8335f/pr-8063 upstream/3.10

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

Push this branch to your fork on GitHub:

$ git push origin patchback/backports/3.10/dfc92967d10eb83a8d726c02c3de90da15f8335f/pr-8063

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 · 2024-02-14T12:35:57Z

@steverep looks like cherry-pick still has conflicts that need to be addressed manually. Please submit a backport PR per instructions above.

…les (aio-libs#8063) Currently server only checks if static routes have a `.gz` extension and serves them with `gzip` encoding. These changes do the same for `.br` files with `br` encoding. Brotli is prioritized over gzip if both exist and are supported by the client, as it should almost always be a smaller content length. I considered making a check for which is smaller if both exist, but figured it wouldn't be worth the extra file system call in the vast majority of cases (at least not for typical web formats). Users should simply use gzip if it's smaller than Brotli for any file. Resolves aio-libs#8062 (cherry picked from commit dfc9296) Co-authored-by: Steve Repsher <[email protected]> Co-authored-by: J. Nick Koston <[email protected]> Co-authored-by: Sviatoslav Sydorenko <[email protected]>

Add server capability to check for Brotli compressed static files

63c1d61

steverep requested review from asvetlov and webknjaz as code owners January 26, 2024 20:26

psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Jan 26, 2024

steverep mentioned this pull request Jan 26, 2024

Use Brotli compression for modern build home-assistant/frontend#17906

Merged

9 tasks