-
Notifications
You must be signed in to change notification settings - Fork 1.5k
wasi-http: Add default-send-request feature flag #11323
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
wasi-http: Add default-send-request feature flag #11323
Conversation
ff2c14e to
5e23189
Compare
dicej
left a comment
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.
Looks great, thanks; just one suggestion inline.
alexcrichton
left a comment
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.
(if you don't mind me doing some review too)
Thanks for this! In addition to the comments below can you add a check on CI that the crate builds without default features? For example somewhere around here
crates/wasi-http/tests/all/p2.rs
Outdated
| #[cfg(feature = "default-send-request")] | ||
| { | ||
| Ok(types::default_send_request(request, config)) | ||
| } | ||
| #[cfg(not(feature = "default-send-request"))] | ||
| { | ||
| Err(ErrorCode::InternalError(Some( | ||
| "default-send-request feature disabled".to_string(), | ||
| )) | ||
| .into()) | ||
| } |
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.
FWIW we generally do not test all possible combinations of features, so I think it's reasonable to leave this out and have it be a compile error if the feature is disabled and tested. My thinking is along the lines of my above comment where I'd prefer to avoid reachable always-return-error functions if possible
5e23189 to
b617075
Compare
b617075 to
ff403ef
Compare
ff403ef to
e2bbde2
Compare
This PR adds a
default-send-requestfeature flag to thewasmtime-wasi-httpcrate that allows disabling the default HTTP request handler implementation and its associated TLS dependencies.