feat: Add support for configuring compression levels #11805
Conversation
rnishtala-sumo
force-pushed
the
compression-config-option2
branch
3 times, most recently
from
7bbcdeb to
ad9e1ee
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11805 +/- ##
==========================================
- Coverage 91.62% 91.60% -0.02%
==========================================
Files 447 447
Lines 23737 23780 +43
==========================================
+ Hits 21749 21784 +35
- Misses 1613 1619 +6
- Partials 375 377 +2 ☔ View full report in Codecov by Sentry. |
rnishtala-sumo
force-pushed
the
compression-config-option2
branch
3 times, most recently
from
b9aedbf to
853d79f
Compare
rnishtala-sumo
force-pushed
the
compression-config-option2
branch
from
853d79f to
ad9a8a3
Compare
| # Use this changelog template to create an entry for release notes. | ||
|
|
||
| # One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' | ||
| change_type: 'breaking' |
There was a problem hiding this comment.
changed this to enhancement.
| # (Optional) One or more lines of additional information to render under the primary note. | ||
| # These lines will be padded with 2 spaces and then inserted directly into the document. | ||
| # Use pipe (|) for multiline entries. | ||
| subtext: |
There was a problem hiding this comment.
Please add more details here describing a new field
There was a problem hiding this comment.
Added some description, PTAL, thanks!
| @@ -104,16 +119,26 @@ func TestHTTPClientCompression(t *testing.T) { | |||
|
|
|||
| req, err := http.NewRequest(http.MethodGet, srv.URL, reqBody) | |||
| require.NoError(t, err, "failed to create request to test handler") | |||
|
|
|||
| compressionType := tt.encoding | |||
| err = compressionType.UnmarshalText([]byte(tt.encoding)) | |||
There was a problem hiding this comment.
Good point, we don't need this, since we already validate this later
| - SpeedFastest: `1` | ||
| - SpeedDefault: `3` | ||
| - SpeedBetterCompression: `6` | ||
| - SpeedBestCompression: `11` |
There was a problem hiding this comment.
Does it mean that zstd has more levels? I see we don't validate that
There was a problem hiding this comment.
zstd doesn't seem to need number validation because it picks the nearest level to whatever number is entered.
config/confighttp/confighttp.go
Outdated
| // Checks the validity of zlib/gzip/flate compression levels | ||
| func isValidLevel(level configcompression.Level) bool { | ||
| return level == zlib.DefaultCompression || | ||
| level == configcompression.LevelNone || |
…iable names, changelog and tests
|
@dmitryax please let me know if there are any other changes that you'd like to see! |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
Description
Add the option of configuring compression levels. This is not intended to be a breaking change for the user.
Link to tracking issue
Issue - #10467
Testing
Unit tests were added/changed