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

Expose compression level on CompressionLayer #245

Closed
Zerowalker opened this issue Apr 7, 2022 · 7 comments · Fixed by #333
Closed

Expose compression level on CompressionLayer #245

Zerowalker opened this issue Apr 7, 2022 · 7 comments · Fixed by #333
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. E-easy Call for participation: Experience needed to fix: Easy / not much E-help-wanted Call for participation: Help is requested to fix this issue.

Comments

@Zerowalker
Copy link

Feature Request

Motivation

Being able to set the compression level will allow one to tweak the settings for optimal speed/compression.
It can be quite a big difference in speed mostly, especially if things are at max.

Proposal

My thoughts are that with_quality is exposed from async_compression.
https://docs.rs/async-compression/latest/src/async_compression/macros.rs.html#19

The interesting one is the Level::Precise so just having access to that should give much of the customization
https://docs.rs/async-compression/latest/src/async_compression/lib.rs.html#208

Alternatives

Haven't tried it but i guess one could manually implement their own compression layer,
or do the compression in the request handler.
I believe it would be quite unnecessary to do that workaround when there's already a CompressionLayer available.

:)

@Nehliin
Copy link
Collaborator

Nehliin commented Apr 8, 2022

PRs are welcome!

@Zerowalker
Copy link
Author

I will try look into it, not sure i can provide code that are up to par though haha.
But i take that as it's not a bad idea, so that's a start:)

@Nehliin
Copy link
Collaborator

Nehliin commented Apr 13, 2022

Happy to help with feedback!

@barvirm
Copy link

barvirm commented Oct 9, 2022

@Nehliin @davidpdrsn Do you have any design ideas? How propagate compression level to the encoders?
The current DecorateAsyncRead is not expandable for these changes.

@Nehliin
Copy link
Collaborator

Nehliin commented Oct 9, 2022

Haven't looked into the details yet. Can look at it next week sometime

@barvirm
Copy link

barvirm commented Oct 14, 2022

@Nehliin Thank you

@davidpdrsn
Copy link
Member

I think a builder method on CompressionLayer and Compression a la

CompressionLayer::new().level(the_level)

is fine.

I think we can copy https://docs.rs/async-compression/latest/async_compression/enum.Level.html into tower-http and internally map between them, to not expose things from async-compression.

@davidpdrsn davidpdrsn added C-feature-request Category: A feature request, i.e: not implemented / a PR. E-easy Call for participation: Experience needed to fix: Easy / not much E-help-wanted Call for participation: Help is requested to fix this issue. labels Dec 2, 2022
rafaelcaricio added a commit to rafaelcaricio/tower-http that referenced this issue Feb 26, 2023
Allows setting the compression quality level for responses.

Fixes tower-rs#245
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. E-easy Call for participation: Experience needed to fix: Easy / not much E-help-wanted Call for participation: Help is requested to fix this issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants