Feature: Request Compression for operations modeled with the @requestCompression trait
#3647
Conversation
This adds a new runtime crate. The new crate contains code related to compressing requests. Not included in this PR is everything needed to actually make use of the new crate. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._
This PR includes all the changes necessary for AWS runtime crates to support request compression. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._
PR 3 of 3. Most of the compression tests happen here since we don't have a request-compression-supporting model in our set of "smoke test" service models. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --------- Co-authored-by: ysaito1001 <awsaito@amazon.com> Co-authored-by: Aaron Todd <aajtodd@users.noreply.github.com>
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
|
||
| impl Compress for Gzip { | ||
| fn compress_bytes(&mut self, bytes: &[u8], writer: &mut dyn Write) -> Result<(), BoxError> { | ||
| Gzip::compress_bytes(self, bytes, writer).map_err(Into::into) |
There was a problem hiding this comment.
Is the flate2 gzip implementation deterministic by default? Was reading and it looks like the GNU version is not unless you use -n since it includes a timestamp in the file. Note sure if that really matters for our use-case.
There was a problem hiding this comment.
The test was broken b/c I mounted the interceptor twice in codegen and I was doing snapshot testing. Once I removed the extra interceptor things worked fine. As far as I can tell, GZIP is deterministic in CI b/c the tests looking for a specific output are passing. If GZIP wasn't deterministic then we'd have to use a decoder to unzip and then check the contents. I think I do that in the compression crate somewhere.
I'm going to ignore it until it actually fails. LMK if you can think of a good test to add.
| // we check to see if the data is big enough to make compression worthwhile. | ||
| if let Some(known_size) = http_body::Body::size_hint(request.body()).exact() { | ||
| if known_size < options.min_compression_size_bytes() as u64 { | ||
| tracing::trace!("request body is below minimum size and will not be compressed"); |
There was a problem hiding this comment.
Can we also print known_size to add additional info for logging, something like "request body of size {known_size} is below ..." (options.min_compression_size_bytes can also be printed here too) ?
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
Motivation and Context
#2891
Description
This PR contains all the changes from the last three compression PRs.
New in this PR:
Checklist
CHANGELOG.next.tomlif I made changes to the smithy-rs codegen or runtime cratesCHANGELOG.next.tomlif I made changes to the AWS SDK, generated SDK code, or SDK runtime cratesBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.