Add Gzip request compression feature #2344
Conversation
wty-Bryant
force-pushed
the
feat-request-compression-2
branch
from
1b5931b
to
07eca61
Compare
wty-Bryant
force-pushed
the
feat-request-compression-2
branch
from
07eca61
to
703c6e9
Compare
config/env_config.go
Outdated
| @@ -370,6 +389,41 @@ func (c EnvConfig) getAppID(context.Context) (string, bool, error) { | |||
| return c.AppID, len(c.AppID) > 0, nil | |||
| } | |||
|
|
|||
| func setRequestMinCompressSizeBytes(bytes **int64) error { | |||
There was a problem hiding this comment.
why are we using a double pointer? **int64?
There was a problem hiding this comment.
this threshold field's default value is not 0, so to distinguish whether or not it has been configured it is represented as a *int64 in config, and thus we need to set it using double pointer
There was a problem hiding this comment.
I would have just used/implemented setInt[64]PtrFromEnv and then inlined the range check, but this is fine.
| }, | ||
| WantErr: true, | ||
| }, | ||
| 45: { |
There was a problem hiding this comment.
whats bad about this case? is there a limit on the size? if so, can we add a comment here specifying that
There was a problem hiding this comment.
yup, the size exceeds max by 1 byte. I can add a comment
| return nil | ||
| } | ||
|
|
||
| func updateDisableRequestCompression(disable **bool, sec ini.Section, key string) error { |
There was a problem hiding this comment.
Same reason above
There was a problem hiding this comment.
FYI you can just use updateBoolPtrFromEnv or whatever it's called.
There was a problem hiding this comment.
updateBoolPtr in sharedconfig doesn't return error for invalid value, I will just keep that
|
can you also generate at least 1 service so we can see. ideally, if there is not a specific service youre testing, i like to generate one vanilla service (like lambda), and S3 |
There was a problem hiding this comment.
for this issue, I actually generate and test cloudwatch.PutMetricData() op with the compression trait, I can publish that first for your testing
…o-v2 into feat-request-compression-2 sync latest change into this branch
sync main into this branch
config/env_config.go
Outdated
| if err != nil { | ||
| return fmt.Errorf("invalid value for env var, %s=%s, need int64", | ||
| awsRequestMinCompressionSizeBytes, b) | ||
| } else if byte < 0 || byte > 10485760 { |
There was a problem hiding this comment.
can you pull (exporting if needed) the const from the smithy-go side?
Add Gzip request compression feature to operations supporting that trait. Insert middleware to those op's stack and enable toggling the compression via client options, env config and shared config profile.