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

Stop using WASI HTTP params in preparation for deprecation #723

Merged
merged 1 commit into from
Aug 30, 2022

Conversation

itowlson
Copy link
Contributor

Relates to #663.

What this PR does:

  • HTTP inbound (trigger):
    • The params field is no longer populated when passed to the guest module. (It is set to an empty vector instead.) As far as I can tell, this should have no impact since the Rust and Go SDKs both appear to consume only the uri, and any query string APIs operate on that not on the params collection.
  • HTTP outbound:
    • Spin will log a warning if an outbound request has any parameters set.
    • The Rust SDK no longer splits the http::Request URI into WASI uri and params. The URI is stringised and preserved intact, and params is set to an empty vector. This should mean the Rust SDK now behaves correctly when the request URI contains a query string but I am not sure where I can test this.
    • I don't believe any changes are required to the Go SDK, as this appears not to refer to params anywhere in the code - I assume it already passed through URIs with query strings unaltered. Again not sure where to put a test for this.

@adamreese can you take a look and confirm that no changes are needed to the Go SDK please? Thanks!

Signed-off-by: itowlson ivan.towlson@fermyon.com

@karthik2804
Copy link
Contributor

I have tested and confirmed that the Rust SDK now behaves correctly when there is a URI containing query strings. I tested by building Bartholomew & handlebars-sprig against the SDK to test the tweeting functionality.

@@ -70,6 +70,10 @@ impl wasi_outbound_http::WasiOutboundHttp for OutboundHttp {
let headers = request_headers(req.headers)?;
let body = req.body.unwrap_or_default().to_vec();

if !req.params.is_empty() {
tracing::log::warn!("HTTP params field is deprecated");
Copy link
Collaborator

Choose a reason for hiding this comment

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

We hardly knew ye.

Signed-off-by: itowlson <ivan.towlson@fermyon.com>
@itowlson itowlson merged commit 139c409 into fermyon:main Aug 30, 2022
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.

4 participants