-
Notifications
You must be signed in to change notification settings - Fork 251
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
Granular route matching #2464
Granular route matching #2464
Conversation
6f6128a
to
c118079
Compare
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.
Style/nit comments; impl looks good
crates/trigger-http/src/lib.rs
Outdated
client_addr: SocketAddr, | ||
) -> Result<Vec<(&'a [&'a str], String)>> { | ||
) -> Result<Vec<([Cow<'static, str>; 2], String)>> { | ||
const fn cowed(strs: &[&'static str; 2]) -> [Cow<'static, str>; 2] { |
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.
FRIGHTEN THE VARIABLES INTO COMPLIANCE WITH YOUR TYPES
crates/trigger-http/src/lib.rs
Outdated
client_addr: SocketAddr, | ||
) -> Result<Vec<(&'a [&'a str], String)>> { | ||
) -> Result<Vec<([Cow<'static, str>; 2], String)>> { |
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.
I would return String
s here. Avoiding allocations is nice and all but all these headers get rewritten a couple of times downstream anyway; probably not worth the extra complexity here.
c118079
to
f53b927
Compare
f53b927
to
8002b28
Compare
Signed-off-by: itowlson <[email protected]>
8002b28
to
bd9e735
Compare
Fixes #1923.
This is WIP, pushed for squigglies if I break the integration tests. It's not yet ready for review.This uses
routefinder
syntax, except we still allow/...
for the trailing wildcard. E.g./users/:userid/notes/...
.The trailing wildcard goes in the
spin-path-info
header, as it does today. Named segment wildcards go in headers along the lines ofspin-path-match-<name>
e.g.spin-path-match-userid
. Feel free to suggest better ideas: I kind of did the first thing I thought of because I knew you reprobates would bikeshed it anyway. grinI am not 100% comfortable with the test coverage, but am not sure what else I can meaningfully add. I'd welcome feedback or suggestions!
Thanks again to @lann and @fibonacci1729 for getting me off an unproductive design and onto this one!