-
Notifications
You must be signed in to change notification settings - Fork 352
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
Add path based routing #751
Conversation
fn split_namespace(host: &str) -> crate::Result<NamespaceName> { | ||
let (ns, _) = host.split_once('.').ok_or_else(|| { | ||
Error::InvalidHost("host header should be in the format <namespace>.<...>".into()) | ||
})?; | ||
let ns = NamespaceName::from_string(ns.to_owned())?; | ||
Ok(ns) | ||
} | ||
|
||
fn extract_namespace(parts: &Parts) -> Option<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.
there is probably an axum way to extract this namespace from the parts, but I could not figure out how to do it
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.
it's simpler than it seems: you need implement Deserialize
for NamespaceName
:
impl<'de> Deserialize for NamespaceName<'de> {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de> {
struct V;
impl<'de> Visitor<'de> for V {
type Value = NamespaceName;
fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "a valid namespace name");
}
fn visit_string<E>(self, v: String) -> Result<Self::Value, E>
where
E: serde::de::Error, {
NamespaceName::from_string(v).map_err(|e| E::custom(e))
}
}
}
}
when you have that, you can use the Path
extractor: https://docs.rs/axum/latest/axum/extract/struct.Path.html
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.
you can now create an extractor, that gets the namespace from path, without parsing anything yourself:
pub struct MakeConnectionExtractorPath<D>(pub Arc<dyn MakeConnection<Connection = D>>);
#[async_trait::async_trait]
impl<F> FromRequestParts<AppState<F>>
for MakeConnectionExtractorPath<<F::Database as Database>::Connection>
where
F: MakeNamespace,
{
type Rejection = Error;
async fn from_request_parts(
parts: &mut Parts,
state: &AppState<F>,
) -> Result<Self, Self::Rejection> {
let auth = Authenticated::from_request_parts(parts, state).await?;
let ns = axum::extract::Path::<NamespaceName>::from_request_parts(parts, state).await?;
Ok(Self(
state
.namespaces
.with_authenticated(ns, auth, |ns| ns.db.connection_maker())
.await?,
))
}
}
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.
finaly, you create a handler:
async fn handle_hrana_pipeline<F: MakeNamespace>(
AxumState(state): AxumState<AppState<F>>,
MakeConnectionExtractorPath(connection_maker): MakeConnectionExtractorPath<
<F::Database as Database>::Connection,
>,
auth: Authenticated,
req: Request<Body>,
) -> Result<Response<Body>, Error> {
Ok(state
.hrana_http_srv
.handle_request(
connection_maker,
auth,
req,
hrana::http::Endpoint::Pipeline,
hrana::Version::Hrana2,
hrana::Encoding::Json,
)
.await?)
}
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.
Thank you so much, I shall try this 🙏
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.
@MarinPostma I have rewritten it and implemented from_request_parts
. It is mostly same as you suggested, one minor change I did was to introduce dynamic hrana version in the path so that same method can be used for both versions.
This PR adds a way to extract namespace from the path. This is done so that users can have smooth experience in
turso dev
and they can connect to multiple namespaces without editing/etc/hosts
file. This patch does not require any changes in shell or libraries./dev/<namespace>
I am not sure if we want to enable to this feature on the platform. So for now, I kept it behind the/dev
sub path/dev/:namespace/v2/pipeline
and/dev/:namespace/v3/pipeline
paths.I have tested this with turso shell and go sdk.