Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: make outgoing request based on wasi-http proposal #2520

Closed

Conversation

eduardomourar
Copy link
Contributor

@eduardomourar eduardomourar commented Mar 31, 2023

Motivation and Context

#2087

This is a continuation of PR #2254, it will replace the mocked HTTP client with one based on the WASI HTTP proposal.

Description

A separate crate called aws-smithy-wasm will be created that contains the HTTP connector. This connector can be used during instantiation of the client SDK in order to route HTTP requests through the WebAssembly host.

Testing

It has been tested with wasmtime v13. At this moment, the integration test only works with WASI (target wasm32-wasi, soon to become wasm32-wasi-preview1). In the context of browsers, an example of usage for a WebAssembly module compiled to WASI can be found here.

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.

@eduardomourar eduardomourar changed the title Feat/default wasi http feat: make outgoing request based on wasi-http proposal Mar 31, 2023
@eduardomourar eduardomourar force-pushed the feat/default-wasi-http branch 4 times, most recently from e493194 to 961a98b Compare April 7, 2023 12:06
@eduardomourar eduardomourar force-pushed the feat/default-wasi-http branch from b4765d8 to cb812de Compare April 23, 2023 21:58
@eduardomourar eduardomourar force-pushed the feat/default-wasi-http branch 3 times, most recently from 9663bfa to dea1d6d Compare May 3, 2023 14:13
@eduardomourar eduardomourar force-pushed the feat/default-wasi-http branch 2 times, most recently from 7f99234 to 448091d Compare May 25, 2023 13:51
@eduardomourar eduardomourar marked this pull request as ready for review May 25, 2023 15:29
@eduardomourar eduardomourar requested review from a team as code owners May 25, 2023 15:29
@eduardomourar eduardomourar force-pushed the feat/default-wasi-http branch 2 times, most recently from dd12391 to d830628 Compare May 26, 2023 12:49
@eduardomourar eduardomourar force-pushed the feat/default-wasi-http branch from c6db7b6 to 917e001 Compare August 30, 2023 14:37
Copy link
Collaborator

@rcoh rcoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not positive we want to be using an unmerged wasmtime PR in our base image—might need to find another way to do it

Just saw it was merged and is pending backport to v13! Awesome!


fn call(&mut self, req: http::Request<SdkBody>) -> Self::Future {
println!("Adapter: sending request...");
let client = DefaultClient::new(None);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume there isn't much complexity though? You'd just store the DefaultClient on the adapter? In any case, I'm fine with this.

rust-runtime/aws-smithy-wasm/src/wasi_adapter.rs Outdated Show resolved Hide resolved
rust-runtime/aws-smithy-wasm/src/wasi_adapter.rs Outdated Show resolved Hide resolved
tools/ci-build/Dockerfile Outdated Show resolved Hide resolved
tools/ci-build/Dockerfile Outdated Show resolved Hide resolved
@eduardomourar
Copy link
Contributor Author

I believe all comments have been addressed from this PR. The only thing missing is preparing the aws-smithy-wasm crate to be published.

@rcoh
Copy link
Collaborator

rcoh commented Oct 9, 2023

Awesome! We'll give this a final read through this week and get it merged. Thanks for your hard work* on this.

@eduardomourar
Copy link
Contributor Author

Awesome! We'll give this a final read through this week and get it merged. Thanks for your hard word on this.

thanks for the patience as well

@rcoh rcoh requested review from a team as code owners November 14, 2023 02:21
@rcoh
Copy link
Collaborator

rcoh commented Jan 10, 2024

what's the status of this? It looks like we meant to merge this but never did for some reason.

@landonxjames
Copy link
Contributor

Looking into this PR since I need something similar. Looks like it will need slightly more than just fixing up the failing tests. Theres an overview in this Zulip thread but the gist of it is that once the PR for stabilizing the wasi 0.2.0 interfaces is merged we can move the WASI dependencies to point to that. From there we can re-implement some of the type conversions from the previously pulled in http_client module. I was talking with @jdisanti about this work last week and I have bandwidth to take it on once the previously mentioned PR is merged.

@rcoh
Copy link
Collaborator

rcoh commented Feb 15, 2024

should this PR be closed in favor of #3409?

@landonxjames
Copy link
Contributor

should this PR be closed in favor of #3409?

Yep, that PR is the continuation of this one but with the finalized WASI 0.2 interfaces

@eduardomourar eduardomourar deleted the feat/default-wasi-http branch February 16, 2024 20:08
github-merge-queue bot pushed a commit that referenced this pull request Feb 28, 2024
## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here -->
This change adds a new crate, `aws-smithy-wasm`, that exports a SDK
compatible WASI http client. This is a continuation of the work in #2520
using the now stabilized WASI 0.2.0 interfaces from the [wasi
crate](https://crates.io/crates/wasi). This supports, but does not
finalize the work for #2087

## Description
<!--- Describe your changes in detail -->

Add a new crate, `aws-smithy-wasm` which exports a function
`wasi_http_client` that will provide the user with a WASI compatible
http client. This client is implemented by using the
`wasi::http::outgoing_handler`
[ref](https://docs.rs/wasi/0.12.0+wasi-0.2.0/wasi/http/outgoing_handler/index.html)
along with some utility implementations of `TryFrom` to transform back
and worth between the types from the `http` crate and the `wasi::http`
types. It also exports a unit struct `WasmSleep` that impls the
`AsyncSleep` trait needed by the SDK.

## Testing
<!--- Please describe in detail how you tested your changes -->
<!--- Include details of your testing environment, and the tests you ran
to -->
<!--- see how your change affects other areas of the code, etc. -->
This is tested via an integration test in
`aws/sdk/integration-tests/webassembly` that uses the wasi http-client
to vuild a config and an operation (that is not sent). It is further
tested in a new canary (`wasm_canary`) that calls the S3
`list_objects_v2` API.

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [X] I have updated `CHANGELOG.next.toml` if I made changes to the
smithy-rs codegen or 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._

---------

Co-authored-by: Eduardo Rodrigues <[email protected]>
Co-authored-by: Eduardo de Moura Rodrigues <[email protected]>
Co-authored-by: ysaito1001 <[email protected]>
Co-authored-by: John DiSanti <[email protected]>
Co-authored-by: Russell Cohen <[email protected]>
Co-authored-by: John DiSanti <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants