-
Notifications
You must be signed in to change notification settings - Fork 247
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
Move HandlerType
to spin-http
#2863
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
use serde::{Deserialize, Serialize}; | ||
use wasmtime::component::Component; | ||
|
||
#[derive(Clone, Debug, Default, Deserialize, Serialize)] | ||
#[serde(deny_unknown_fields)] | ||
|
@@ -11,3 +12,67 @@ pub struct Metadata { | |
pub fn default_base() -> String { | ||
"/".into() | ||
} | ||
|
||
/// The type of http handler export used by a component. | ||
#[derive(Clone, Copy)] | ||
pub enum HandlerType { | ||
Spin, | ||
Wagi, | ||
Wasi0_2, | ||
Wasi2023_11_10, | ||
Wasi2023_10_18, | ||
} | ||
|
||
/// The `incoming-handler` export for `wasi:http` version rc-2023-10-18 | ||
pub const WASI_HTTP_EXPORT_2023_10_18: &str = "wasi:http/[email protected]"; | ||
/// The `incoming-handler` export for `wasi:http` version rc-2023-11-10 | ||
pub const WASI_HTTP_EXPORT_2023_11_10: &str = "wasi:http/[email protected]"; | ||
/// The `incoming-handler` export for `wasi:http` version 0.2.0 | ||
pub const WASI_HTTP_EXPORT_0_2_0: &str = "wasi:http/[email protected]"; | ||
/// The `incoming-handler` export for `wasi:http` version 0.2.1 | ||
pub const WASI_HTTP_EXPORT_0_2_1: &str = "wasi:http/[email protected]"; | ||
/// The `inbound-http` export for `fermyon:spin` | ||
pub const SPIN_HTTP_EXPORT: &str = "fermyon:spin/inbound-http"; | ||
|
||
impl HandlerType { | ||
/// Determine the handler type from the exports of a component. | ||
pub fn from_component( | ||
engine: &wasmtime::Engine, | ||
component: &Component, | ||
) -> anyhow::Result<HandlerType> { | ||
let mut handler_ty = None; | ||
|
||
let mut set = |ty: HandlerType| { | ||
if handler_ty.is_none() { | ||
handler_ty = Some(ty); | ||
Ok(()) | ||
} else { | ||
Err(anyhow::anyhow!( | ||
"component exports multiple different handlers but \ | ||
it's expected to export only one" | ||
)) | ||
} | ||
}; | ||
let ty = component.component_type(); | ||
for (name, _) in ty.exports(engine) { | ||
match name { | ||
WASI_HTTP_EXPORT_2023_10_18 => set(HandlerType::Wasi2023_10_18)?, | ||
WASI_HTTP_EXPORT_2023_11_10 => set(HandlerType::Wasi2023_11_10)?, | ||
WASI_HTTP_EXPORT_0_2_0 | WASI_HTTP_EXPORT_0_2_1 => set(HandlerType::Wasi0_2)?, | ||
SPIN_HTTP_EXPORT => set(HandlerType::Spin)?, | ||
_ => {} | ||
} | ||
} | ||
|
||
handler_ty.ok_or_else(|| { | ||
anyhow::anyhow!( | ||
"Expected component to export one of \ | ||
`{WASI_HTTP_EXPORT_2023_10_18}`, \ | ||
`{WASI_HTTP_EXPORT_2023_11_10}`, \ | ||
`{WASI_HTTP_EXPORT_0_2_0}`, \ | ||
`{WASI_HTTP_EXPORT_0_2_1}`, \ | ||
or `{SPIN_HTTP_EXPORT}` but it exported none of those" | ||
) | ||
}) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,14 +20,14 @@ use spin_http::{ | |
body, | ||
config::{HttpExecutorType, HttpTriggerConfig}, | ||
routes::{RouteMatch, Router}, | ||
trigger::HandlerType, | ||
}; | ||
use tokio::{ | ||
io::{AsyncRead, AsyncWrite}, | ||
net::TcpListener, | ||
task, | ||
}; | ||
use tracing::Instrument; | ||
use wasmtime::component::Component; | ||
use wasmtime_wasi_http::body::HyperOutgoingBody; | ||
|
||
use crate::{ | ||
|
@@ -104,7 +104,7 @@ impl<F: RuntimeFactors> HttpServer<F> { | |
let handler_type = match &trigger_config.executor { | ||
None | Some(HttpExecutorType::Http) => { | ||
let component = trigger_app.get_component(component_id)?; | ||
HandlerType::from_component(trigger_app.engine(), component)? | ||
HandlerType::from_component(trigger_app.engine().as_ref(), component)? | ||
} | ||
Some(HttpExecutorType::Wagi(wagi_config)) => { | ||
anyhow::ensure!( | ||
|
@@ -464,61 +464,3 @@ pub(crate) trait HttpExecutor: Clone + Send + Sync + 'static { | |
client_addr: SocketAddr, | ||
) -> impl Future<Output = anyhow::Result<Response<Body>>>; | ||
} | ||
|
||
/// Whether this handler uses the custom Spin http handler interface for wasi-http | ||
#[derive(Clone, Copy)] | ||
pub enum HandlerType { | ||
Spin, | ||
Wagi, | ||
Wasi0_2, | ||
Wasi2023_11_10, | ||
Wasi2023_10_18, | ||
} | ||
|
||
pub const WASI_HTTP_EXPORT_2023_10_18: &str = "wasi:http/[email protected]"; | ||
pub const WASI_HTTP_EXPORT_2023_11_10: &str = "wasi:http/[email protected]"; | ||
pub const WASI_HTTP_EXPORT_0_2_0: &str = "wasi:http/[email protected]"; | ||
pub const WASI_HTTP_EXPORT_0_2_1: &str = "wasi:http/[email protected]"; | ||
|
||
impl HandlerType { | ||
/// Determine the handler type from the exports of a component | ||
pub fn from_component( | ||
engine: impl AsRef<wasmtime::Engine>, | ||
component: &Component, | ||
) -> anyhow::Result<HandlerType> { | ||
let mut handler_ty = None; | ||
|
||
let mut set = |ty: HandlerType| { | ||
if handler_ty.is_none() { | ||
handler_ty = Some(ty); | ||
Ok(()) | ||
} else { | ||
Err(anyhow::anyhow!( | ||
"component exports multiple different handlers but \ | ||
it's expected to export only one" | ||
)) | ||
} | ||
}; | ||
let ty = component.component_type(); | ||
for (name, _) in ty.exports(engine.as_ref()) { | ||
match name { | ||
WASI_HTTP_EXPORT_2023_10_18 => set(HandlerType::Wasi2023_10_18)?, | ||
WASI_HTTP_EXPORT_2023_11_10 => set(HandlerType::Wasi2023_11_10)?, | ||
WASI_HTTP_EXPORT_0_2_0 | WASI_HTTP_EXPORT_0_2_1 => set(HandlerType::Wasi0_2)?, | ||
"fermyon:spin/inbound-http" => set(HandlerType::Spin)?, | ||
_ => {} | ||
} | ||
} | ||
|
||
handler_ty.ok_or_else(|| { | ||
anyhow::anyhow!( | ||
"Expected component to export one of \ | ||
`{WASI_HTTP_EXPORT_2023_10_18}`, \ | ||
`{WASI_HTTP_EXPORT_2023_11_10}`, \ | ||
`{WASI_HTTP_EXPORT_0_2_0}`, \ | ||
`{WASI_HTTP_EXPORT_0_2_1}`, \ | ||
or `fermyon:spin/inbound-http` but it exported none of those" | ||
) | ||
}) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should, I think, be under the
runtime
feature. We have consumers of this crate who use it for things like route parsing (e.g. the cloud plugin), but do not want to drag the whole of wasmtime along with it.(I'm happy if you prefer to reorganise things to avoid the feature thing, but I really really want a way for cloud-plugin to get what it needs without including wasmtime. Whether that is features or separate crates, I don't care, but it makes a big difference!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I just tried updating cloud-plugin and it looks like some of the other recebt changes have also brought in wasmtime dependency. I'll get a PR out for all of it together.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I did some more investigation and I'm gonna punt on it for now. The wasmtime dependency makes 1 min of difference to a clean cloud-plugin build, but not enough difference to an incremental build to be worth the candle, and we are churning cloud-plugin dependencies a lot more slowly than when it drove me to extremes. Something to maybe think about how we can streamline in a more sustainable way in future, but not worth fretting about for now.
Sorry for bending your ear @rylev!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a really good point, and something I would like to start getting a better handle on. Even though it doesn't make a huge difference, I can take a look at fixing this.