Http updates #3059

rcoh · 2023-10-11T16:22:34Z

Motivation and Context

Upgrade guidance
changelog

To allow seamless adoption of the incoming http crate, we're updating our client crates to use an intermediate type.

Description

Update client code to use HttpRequest. Some internal code still uses http::Request but those usages are limited.

Testing

CI

Checklist

I have updated CHANGELOG.next.toml if I made changes to the smithy-rs codegen or runtime crates
I have updated CHANGELOG.next.toml if I made changes to the AWS SDK, generated SDK code, or SDK runtime crates

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

github-actions · 2023-10-11T16:41:01Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

github-actions · 2023-10-11T20:47:00Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

github-actions · 2023-10-12T16:34:35Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
Server Test (ignoring whitespace)
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

github-actions · 2023-10-12T18:15:26Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
Server Test (ignoring whitespace)
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

github-actions · 2023-10-12T18:52:50Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
Server Test (ignoring whitespace)
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

github-actions · 2023-10-12T19:18:52Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
Server Test (ignoring whitespace)
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

github-actions · 2023-10-12T20:31:22Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
Server Test (ignoring whitespace)
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

jdisanti

Great work on this! I only have minor comments so far.

jdisanti · 2023-10-12T20:26:17Z

aws/rust-runtime/aws-inlineable/src/presigning.rs

        self.0.uri()
    }

    /// Returns any HTTP headers that need to go along with the request, except for `Host`,
    /// which should be sent based on the endpoint in the URI by the HTTP client rather than
    /// added directly.
-    pub fn headers(&self) -> &http::HeaderMap<http::HeaderValue> {
-        self.0.headers()
+    pub fn headers(&self) -> impl Iterator<Item = (&str, &str)> {


Do we need the sensitive flag on headers for presigning?

my take is this header is probably not that sensitive to make it worthwhile to return a custom type here. Open to thoughts though.

jdisanti · 2023-10-12T20:31:55Z

aws/sdk/integration-tests/s3/tests/endpoints.rs

@@ -3,6 +3,8 @@
 * SPDX-License-Identifier: Apache-2.0
 */

+#![cfg(feature = "test-util")]


Now that integration tests are predicated on features, we should update the check-aws-sdk-standalone-integration-tests script here:

pushd "${tmp_dir}/aws/sdk/integration-tests" cargo check --tests popd

It should check with --all-features.

jdisanti · 2023-10-12T20:37:05Z

codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/util/Strings.kt

@@ -18,10 +18,10 @@ fun String.doubleQuote(): String =
 */
 fun String.dq(): String = this.doubleQuote()

+val completeWords: List<String> = listOf("ipv4", "ipv6", "sigv4", "mib", "gib", "kib", "ttl")


Suggested change

val completeWords: List<String> = listOf("ipv4", "ipv6", "sigv4", "mib", "gib", "kib", "ttl")

private val completeWords: List<String> = listOf("ipv4", "ipv6", "sigv4", "mib", "gib", "kib", "ttl")

jdisanti · 2023-10-12T20:38:28Z

rust-runtime/aws-smithy-checksums/src/http.rs

I wonder why GitHub thinks this file is binary 🤔

Please see #3778. The original author put non-printable characters into this file.

jdisanti · 2023-10-12T20:42:16Z

aws/rust-runtime/aws-inlineable/src/presigning.rs

@@ -168,62 +170,46 @@ impl PresigningConfigBuilder {
 ///
 /// **This struct has conversion convenience functions:**
 ///
-/// - [`PresignedRequest::to_http_request<B>`][Self::to_http_request] returns an [`http::Request<B>`](https://docs.rs/http/0.2.6/http/request/struct.Request.html)
+/// - [`PresignedRequest::to_http_02x_request<B>`][Self::to_http_03x_request] returns an [`http::Request<B>`](https://docs.rs/http/0.2.6/http/request/struct.Request.html)


Suggested change

/// - [`PresignedRequest::to_http_02x_request<B>`][Self::to_http_03x_request] returns an [`http::Request<B>`](https://docs.rs/http/0.2.6/http/request/struct.Request.html)

/// - [`PresignedRequest::to_http_02x_request<B>`][Self::to_http_02x_request] returns an [`http::Request<B>`](https://docs.rs/http/0.2.6/http/request/struct.Request.html)

aws/rust-runtime/aws-inlineable/src/presigning.rs

rust-runtime/aws-smithy-runtime/src/client/http/test_util/dvr.rs

jdisanti · 2023-10-12T20:44:39Z

rust-runtime/aws-smithy-runtime/src/client/http/test_util/replay.rs

@@ -29,8 +29,11 @@ pub struct ReplayEvent {

 impl ReplayEvent {
    /// Creates a new `ReplayEvent`.
-    pub fn new(request: HttpRequest, response: HttpResponse) -> Self {
-        Self { request, response }
+    pub fn new(request: impl TryInto<HttpRequest>, response: impl TryInto<HttpResponse>) -> Self {


rust-runtime/aws-smithy-runtime/src/client/orchestrator/endpoints.rs

github-actions · 2023-10-13T16:56:05Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
Server Test (ignoring whitespace)
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

ysaito1001

Lots of places to update. Thanks for the work 🙇

ysaito1001 · 2023-10-13T17:46:27Z

aws/sdk/integration-tests/polly/tests/presigning.rs

@@ -55,5 +56,5 @@ async fn test_presigning() {
        ][..],
        &query_params
    );
-    assert!(presigned.headers().is_empty());
+    assert_eq!(presigned.headers().count(), 0);


Suggested change

assert_eq!(presigned.headers().count(), 0);

assert_eq!(0, presigned.headers().count());

ysaito1001 · 2023-10-13T17:48:28Z

aws/sdk/integration-tests/s3/tests/presigning.rs

@@ -97,7 +100,7 @@ async fn test_presigning() {
        ][..],
        &query_params
    );
-    assert!(presigned.headers().is_empty());
+    assert_eq!(presigned.headers().count(), 0);


Suggested change

assert_eq!(presigned.headers().count(), 0);

assert_eq!(0, presigned.headers().count());

ysaito1001 · 2023-10-13T17:50:03Z

aws/sdk/integration-tests/s3/tests/presigning.rs

+    assert_eq!(
+        headers.get(CONTENT_TYPE.as_str()),
+        Some(&"application/x-test")
+    );


Suggested change

assert_eq!(

headers.get(CONTENT_TYPE.as_str()),

Some(&"application/x-test")

);

assert_eq!(

Some(&"application/x-test"),

headers.get(CONTENT_TYPE.as_str())

);

ysaito1001 · 2023-10-13T17:50:42Z

aws/sdk/integration-tests/s3/tests/presigning.rs

+        headers.get(CONTENT_TYPE.as_str()),
+        Some(&"application/x-test")
+    );
+    assert_eq!(headers.get(CONTENT_LENGTH.as_str()), Some(&"12345"));


Suggested change

assert_eq!(headers.get(CONTENT_LENGTH.as_str()), Some(&"12345"));

assert_eq!(Some(&"12345"), headers.get(CONTENT_LENGTH.as_str()));

ysaito1001 · 2023-10-13T17:50:57Z

aws/sdk/integration-tests/s3/tests/presigning.rs

+        Some(&"application/x-test")
+    );
+    assert_eq!(headers.get(CONTENT_LENGTH.as_str()), Some(&"12345"));
+    assert_eq!(headers.len(), 2);


Suggested change

assert_eq!(headers.len(), 2);

assert_eq!(2, headers.len());

ysaito1001 · 2023-10-13T17:55:42Z

rust-runtime/aws-smithy-http/src/query_writer.rs

@@ -15,6 +16,11 @@ pub struct QueryWriter {
 }

 impl QueryWriter {
+    /// Creates a new `QueryWriter` from a string
+    pub fn new_from_string(uri: &str) -> Result<Self, InvalidUri> {


Just curious, why not impl FromStr for QueryWriter ?

github-actions · 2023-10-13T20:09:04Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
Server Test (ignoring whitespace)
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

For the same reason that the request wrapper types were created in #3059, this PR establishes the response wrapper types and refactors existing code to use them. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._

## Motivation and Context In #3059 there was an accidental deletion of a test ## Description Re-introduce checking the URL in protocol tests ## Checklist  - [ ] I have updated `CHANGELOG.next.toml` if I made changes to the smithy-rs codegen or runtime crates - [ ] I have updated `CHANGELOG.next.toml` if I made changes to the AWS SDK, generated SDK code, or SDK runtime crates ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._

rcoh force-pushed the http-updates branch from 8927346 to 60b6a52 Compare October 11, 2023 20:28

rcoh force-pushed the http-updates branch from bcb089d to 6265246 Compare October 12, 2023 16:16

rcoh force-pushed the http-updates branch from 0996ce6 to 62c93ed Compare October 12, 2023 19:01

rcoh force-pushed the http-updates branch from 62c93ed to bd9548f Compare October 12, 2023 20:12

jdisanti reviewed Oct 12, 2023

View reviewed changes

rcoh marked this pull request as ready for review October 13, 2023 16:37

rcoh requested review from a team as code owners October 13, 2023 16:37

rcoh force-pushed the http-updates branch from 0fb1d5f to 7b16541 Compare October 13, 2023 16:38

rcoh requested a review from jdisanti October 13, 2023 16:39

rcoh added needs-sdk-review needs-server-review labels Oct 13, 2023

rcoh added the breaking-change This will require a breaking change label Oct 13, 2023

jdisanti approved these changes Oct 13, 2023

View reviewed changes

rcoh added this pull request to the merge queue Oct 13, 2023

github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Oct 13, 2023

ysaito1001 approved these changes Oct 13, 2023

View reviewed changes

rcoh added 5 commits October 13, 2023 15:32

Update aws-smithy-runtime-api

12e6d3c

Update all rust-runtime crates

a237275

Update all aws/rust-runtime crates

d0d4e6f

STS code generates end-to-end and tests pass

ac711c8

Fix client protocol tests

eb3d638

rcoh added 11 commits October 13, 2023 15:50

Update AWS Integration Tests to compile

d498068

Sdk compiling, all features, all targets

482ef93

AWS Integration tests passing

1a9bf44

Clear clippy lints

f087b9f

Fix codegen server unit tests

fff1c24

Fix SDK/client unit tests

e8cf4c2

Fix broken doc links, name into_http function properly

da17294

Clear out external-types issue

3e03161

Fix server E2E tests

14387f2

CR feedback

1fb1b7a

Changelog updates

d517487

rcoh force-pushed the http-updates branch from 7b16541 to d517487 Compare October 13, 2023 19:51

rcoh added this pull request to the merge queue Oct 14, 2023

Merged via the queue into main with commit 684605a Oct 14, 2023
40 of 41 checks passed

rcoh deleted the http-updates branch October 14, 2023 01:23

Velfi removed needs-sdk-review needs-server-review labels Oct 19, 2023

rcoh mentioned this pull request Oct 25, 2023

0.57 release tracking issue #3097

Closed

9 tasks

jdisanti mentioned this pull request Nov 7, 2023

Create the HTTP Response wrapper types #3148

Merged

rcoh mentioned this pull request Nov 16, 2023

Fix a bug where we aren't checking the URL #3211

Merged

2 tasks

NickDarvey mentioned this pull request Jun 25, 2024

chore: upgrade aws-config for aarch64-pc-windows support lancedb/lance#2510

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Http updates #3059

Http updates #3059

rcoh commented Oct 11, 2023 •

edited

Loading

github-actions bot commented Oct 11, 2023

github-actions bot commented Oct 11, 2023

github-actions bot commented Oct 12, 2023

github-actions bot commented Oct 12, 2023

github-actions bot commented Oct 12, 2023

github-actions bot commented Oct 12, 2023

github-actions bot commented Oct 12, 2023

jdisanti left a comment

jdisanti Oct 12, 2023

rcoh Oct 13, 2023

jdisanti Oct 12, 2023

jdisanti Oct 12, 2023

jdisanti Oct 12, 2023

dtolnay Aug 1, 2024

jdisanti Oct 12, 2023

jdisanti Oct 12, 2023

github-actions bot commented Oct 13, 2023

ysaito1001 left a comment

ysaito1001 Oct 13, 2023

ysaito1001 Oct 13, 2023

ysaito1001 Oct 13, 2023

ysaito1001 Oct 13, 2023

ysaito1001 Oct 13, 2023

ysaito1001 Oct 13, 2023

github-actions bot commented Oct 13, 2023

	val completeWords: List<String> = listOf("ipv4", "ipv6", "sigv4", "mib", "gib", "kib", "ttl")
	private val completeWords: List<String> = listOf("ipv4", "ipv6", "sigv4", "mib", "gib", "kib", "ttl")

	/// - [`PresignedRequest::to_http_02x_request<B>`][Self::to_http_03x_request] returns an [`http::Request<B>`](https://docs.rs/http/0.2.6/http/request/struct.Request.html)
	/// - [`PresignedRequest::to_http_02x_request<B>`][Self::to_http_02x_request] returns an [`http::Request<B>`](https://docs.rs/http/0.2.6/http/request/struct.Request.html)

	assert_eq!(presigned.headers().count(), 0);
	assert_eq!(0, presigned.headers().count());

	assert_eq!(headers.get(CONTENT_LENGTH.as_str()), Some(&"12345"));
	assert_eq!(Some(&"12345"), headers.get(CONTENT_LENGTH.as_str()));

Http updates #3059

Http updates #3059

Conversation

rcoh commented Oct 11, 2023 • edited Loading

Motivation and Context

Description

Testing

Checklist

github-actions bot commented Oct 11, 2023

github-actions bot commented Oct 11, 2023

github-actions bot commented Oct 12, 2023

github-actions bot commented Oct 12, 2023

github-actions bot commented Oct 12, 2023

github-actions bot commented Oct 12, 2023

github-actions bot commented Oct 12, 2023

jdisanti left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Oct 13, 2023

ysaito1001 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Oct 13, 2023

rcoh commented Oct 11, 2023 •

edited

Loading