HTTP Wrapper Type RFC & Implementation #2912

rcoh · 2023-08-09T17:03:14Z

Motivation and Context

Implementation of the HTTP request wrapper type from RFC

Description

RFC + implementation of HttpRequest

Testing

IT/UT
Separate PR which uses this type in implementation of the SDK

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

jdisanti

This looks great so far!

rust-runtime/aws-smithy-runtime-api/src/client/http/request.rs

jdisanti · 2023-08-09T17:37:51Z

rust-runtime/aws-smithy-runtime-api/src/client/http/request.rs

+    ///
+    /// Depending on the internal storage type, this operation may be free or it may have an internal
+    /// cost.
+    pub fn into_http0x(self) -> http::Request<B> {


Hypothetically, if there was ever a http 0.3 release before 1.0, this would need an additional method that would break the naming convention, right?

yeah, we should either make this a From impl or name it more precisely

Should it be fallible in case it's not possible to convert in the future (due to losing information or something)?

jdisanti · 2023-08-09T17:38:31Z

rust-runtime/aws-smithy-runtime-api/src/client/http/request.rs

+    }
+
+    /// Returns the body associated with the request
+    pub fn body(&self) -> &B {


Would need body_mut too.

Also, it would be super nice to have a conveniences like map_body/take_body since we frequently have to juggle mem::swaps in the orchestrator code to accomplish that.

yeah there are a bunch of methods that need to be added to support everything

ysaito1001

Looks good so far! I originally had a question whether

impl TryFrom<http0::HeaderValue> for HeaderValue

is considered exposing a 3rd party type in public API, but this should be ok as it's done in a way that is not ossified, as called out in the RFC.

github-actions · 2023-08-14T16:55:42Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
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-08-14T17:44:43Z

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-08-17T14:13:03Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
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.

rust-runtime/aws-smithy-runtime-api/src/client/http/request.rs

design/src/rfcs/rfc0037_http_wrapper.md

rust-runtime/aws-smithy-runtime-api/src/client/http/mod.rs

jdisanti · 2023-08-17T17:11:28Z

rust-runtime/aws-smithy-runtime-api/src/client/http/request.rs

+
+#[derive(Debug)]
+/// An HTTP Request Type
+pub struct Request<B = SdkBody> {


Have you tried using the Request/Response with Surf? Does it play nicely with that API?

the API works fine except for extensions. We'd need to maintain a copy of each extension map we aim to target or the maps would need to support construction from TypeId directly

jdisanti · 2023-08-17T17:12:15Z

rust-runtime/aws-smithy-runtime-api/src/client/http/request.rs

+
+/// A Request URI
+#[derive(Debug, Clone)]
+pub struct Uri {


Would love to have nice URI manipulations on this in the future 😄

jdisanti · 2023-08-17T17:14:18Z

rust-runtime/aws-smithy-runtime-api/src/client/http/request.rs

+    }
+
+    /// Adds an extension to the request extensions
+    pub fn add_extension<T: Send + Sync + Clone + 'static>(&mut self, extension: T) {


Should this be fallible in case the underlying HTTP library doesn't support extensions?

so currently we have fallibility in the conversion to the final request type—you actually don't know what the final request type is going to be until the request is dispatched.

We can probably refactor at some point to remove the need to set request extensions and instead handle it during the conversion to http03x::Request...not a bad idea actually, hmmm

jdisanti · 2023-08-17T17:15:52Z

design/src/rfcs/rfc0037_http_wrapper.md

+        key: impl AsHeaderName,
+        value: impl AsHeaderName,


Suggested change

key: impl AsHeaderName,

value: impl AsHeaderName,

key: impl AsHeaderComponent,

value: impl AsHeaderComponent,

rust-runtime/aws-smithy-runtime-api/src/client/http/request.rs

github-actions · 2023-08-28T17:05:26Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
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.

design/src/rfcs/rfc0037_http_wrapper.md

github-actions · 2023-08-29T14:05:17Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
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.

Co-authored-by: John DiSanti <[email protected]>

github-actions · 2023-09-05T19:31:59Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
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.

Co-authored-by: John DiSanti <[email protected]>

…p-type-rfc

github-actions · 2023-09-06T20:34:38Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
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-09-07T18:13:50Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
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.

jdisanti · 2023-09-26T19:30:11Z

rust-runtime/aws-smithy-runtime-api/src/client/http/request.rs

+        let http0 = req.into_http03x().unwrap();
+        assert_eq!(http0.uri(), "http://bar.com");
+    }
+


Just note somewhere that there are more tests to add. For example, the Uri::set_endpoint method.

github-actions · 2023-09-26T19:34:50Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
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-10T12:55:11Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
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.

jdisanti reviewed Aug 9, 2023

View reviewed changes

ysaito1001 reviewed Aug 9, 2023

View reviewed changes

rcoh force-pushed the http-type-rfc branch from 655ecaf to 692a953 Compare August 14, 2023 16:28

rcoh marked this pull request as ready for review August 14, 2023 16:29

rcoh requested review from a team as code owners August 14, 2023 16:29

rcoh force-pushed the http-type-rfc branch from 692a953 to 7423557 Compare August 14, 2023 17:08

rcoh added the needs-sdk-review label Aug 14, 2023

rcoh force-pushed the http-type-rfc branch 2 times, most recently from f4755f4 to aa1bbc1 Compare August 17, 2023 13:44

rcoh changed the title ~~WIP RFC for comment~~ HTTP Wrapper Type RFC & Implementation Aug 17, 2023

jdisanti reviewed Aug 17, 2023

View reviewed changes

jdisanti reviewed Aug 28, 2023

View reviewed changes

design/src/rfcs/rfc0037_http_wrapper.md Outdated Show resolved Hide resolved

rcoh added 3 commits August 29, 2023 09:38

RFC for Http Wrapper Types & implementation

d6d5cba

cleanups & feedback

096606b

Update header name description

e83b3ca

rcoh force-pushed the http-type-rfc branch from 3ba959c to e83b3ca Compare August 29, 2023 13:38

rcoh added 3 commits September 5, 2023 14:59

RFC for Http Wrapper Types & implementation

0741da5

cleanups & feedback

2f81cf7

Update header name description

4a9b77e

rcoh force-pushed the http-type-rfc branch from e83b3ca to 4a9b77e Compare September 5, 2023 18:59

Update design/src/rfcs/rfc0037_http_wrapper.md

147924b

Co-authored-by: John DiSanti <[email protected]>

rcoh and others added 2 commits September 6, 2023 14:26

Rename field in RFC

58d2afe

Update rust-runtime/aws-smithy-runtime-api/src/client/http/mod.rs

9449842

Co-authored-by: John DiSanti <[email protected]>

rcoh added 3 commits September 6, 2023 15:45

RFC updates

0d24f27

Merge branch 'http-type-rfc' of github.com:awslabs/smithy-rs into htt…

87c2414

…p-type-rfc

Add updates from implementation

8f51492

rcoh force-pushed the http-type-rfc branch from f49cad0 to 8f51492 Compare September 6, 2023 20:00

Merge branch 'main' of github.com:awslabs/smithy-rs into http-type-rfc

f8ba166

Merge branch 'main' of github.com:awslabs/smithy-rs into http-type-rfc

356582c

Merge branch 'main' into http-type-rfc

2d2a6d7

rcoh requested a review from jdisanti September 26, 2023 19:18

jdisanti approved these changes Sep 26, 2023

View reviewed changes

rcoh enabled auto-merge September 26, 2023 19:30

rcoh added 3 commits October 9, 2023 14:39

Increase http minimal version

3482014

Merge branch 'main' of github.com:awslabs/smithy-rs into http-type-rfc

d2d320f

Fix bad merge

22ec2ad

rcoh force-pushed the http-type-rfc branch from cd0d91e to 22ec2ad Compare October 9, 2023 20:19

Remove public TryFrom impl

6c04ede

rcoh removed the needs-sdk-review label Oct 10, 2023

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

Merged via the queue into main with commit 7b30ffc Oct 10, 2023
40 of 41 checks passed

rcoh deleted the http-type-rfc branch October 10, 2023 13:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HTTP Wrapper Type RFC & Implementation #2912

HTTP Wrapper Type RFC & Implementation #2912

rcoh commented Aug 9, 2023 •

edited

Loading

jdisanti left a comment

jdisanti Aug 9, 2023

rcoh Aug 9, 2023

jdisanti Aug 9, 2023

jdisanti Aug 9, 2023

rcoh Aug 9, 2023

ysaito1001 left a comment

github-actions bot commented Aug 14, 2023

github-actions bot commented Aug 14, 2023

github-actions bot commented Aug 17, 2023

jdisanti Aug 17, 2023

rcoh Sep 6, 2023

jdisanti Aug 17, 2023

jdisanti Aug 17, 2023

rcoh Aug 29, 2023

jdisanti Aug 17, 2023

github-actions bot commented Aug 28, 2023

github-actions bot commented Aug 29, 2023

github-actions bot commented Sep 5, 2023

github-actions bot commented Sep 6, 2023

github-actions bot commented Sep 7, 2023

jdisanti Sep 26, 2023

github-actions bot commented Sep 26, 2023

github-actions bot commented Oct 10, 2023

HTTP Wrapper Type RFC & Implementation #2912

HTTP Wrapper Type RFC & Implementation #2912

Conversation

rcoh commented Aug 9, 2023 • edited Loading

Motivation and Context

Description

Testing

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

ysaito1001 left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 14, 2023

github-actions bot commented Aug 14, 2023

github-actions bot commented Aug 17, 2023

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 Aug 28, 2023

github-actions bot commented Aug 29, 2023

github-actions bot commented Sep 5, 2023

github-actions bot commented Sep 6, 2023

github-actions bot commented Sep 7, 2023

Choose a reason for hiding this comment

github-actions bot commented Sep 26, 2023

github-actions bot commented Oct 10, 2023

rcoh commented Aug 9, 2023 •

edited

Loading