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

CompressMiddleware - Supporting ZStd, Brotli, GZip #2564

Closed
wants to merge 6 commits into from

Conversation

Zaczero
Copy link

@Zaczero Zaczero commented Apr 3, 2024

Summary

I have always felt like a more robust compression middleware is needed for Starlette. While working one one of my projects, I implemented a robust compression class that supports more than just GZip and has more sane defaults. I decided that it's worth pushing up the upstream so other people can benefit from it.

Today I gave it some polish so it's ready for use in other projects.

I can add markdown documentation when there is a green light for this change. I don't want to put too much effort in something without guarantee.

Key points

  • The class has 2 new dependencies, brotli (for CPython) or brotlicffi (for non-CPython), and zstandard which has built-in cffi-checking logic.
  • Default compression level was reduced to 4 (from 9 in GZipMiddleware). Higher levels have very little compression ratio benefit compared to substantially increased response time. For dynamic responses values below 5 make the most sense.
  • I have compared my implementation with GZipMiddleware to make sure I don't miss any edge-cases (although I am still just a human).
  • I copied all tests from GZipMiddleware and added more to increase code coverage.

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.

@Zaczero
Copy link
Author

Zaczero commented Apr 3, 2024

Contributor feedback: I wish the CI runs did not abort each other on first fail, and actually run till the end to produce as much useful information as possible. It's slightly annoying to fix problems one-by-one and then wait few minutes for CI to progress further. For example here, I was not aware of 3.8 issues because 3.11 job aborted it.

@Kludex
Copy link
Sponsor Member

Kludex commented Apr 17, 2024

I think this middleware would be great as a third party package.

We can then document it on the https://www.starlette.io/third-party-packages/ page.

@Zaczero
Copy link
Author

Zaczero commented Apr 17, 2024

Sounds good. I'll work on it and then open another PR with the documentation update. If that's fine, I'll close this PR in about a week or two.

@Kludex
Copy link
Sponsor Member

Kludex commented Apr 20, 2024

Sounds good. I'll work on it and then open another PR with the documentation update. If that's fine, I'll close this PR in about a week or two.

I'll close it now for housekeeping, but the PR for updating the docs is more than welcome. :)

@Kludex Kludex closed this Apr 20, 2024
@Kludex
Copy link
Sponsor Member

Kludex commented May 1, 2024

@Zaczero Do you mind if I create the third party package myself?

@Zaczero
Copy link
Author

Zaczero commented May 1, 2024

@Kludex Slightly yes because I need some more projects for my CV :-) I am getting back home (from 2 weeks away) tomorrow and I'll be able to continue with this topic - I didn't forget about it. Alternatively, we could create a repo where we both hold write perms, I wouldn't mind it. It would be a good opportunity to learn something new from each other. But then, wouldn't it be an overkill.

@Kludex
Copy link
Sponsor Member

Kludex commented May 1, 2024

I can wait. Let me know when ready so I can recommend.

@Zaczero
Copy link
Author

Zaczero commented May 7, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants