-
Notifications
You must be signed in to change notification settings - Fork 5
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
refactor(graph-gateway): add query selector extractor and check #561
Conversation
// This is very useful for investigating gateway logs in production | ||
let selector = match &resolved_deployments { | ||
Ok((_, Some(subgraph))) => subgraph.id.to_string(), | ||
Ok((deployments, None)) => deployments | ||
.iter() | ||
.map(|d| d.id.to_string()) | ||
.collect::<Vec<_>>() | ||
.join(","), | ||
Err(_) => "".to_string(), | ||
}; | ||
span.record("selector", tracing::field::display(selector)); |
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 code snippet has been "moved" to the QuerySelector
extractor:
This is equivalent since we are setting either the SubgraphId
or the DeploymentId
depending on the selector.
Aiming to merge this post v17.1.0 release |
07076bc
to
6542b59
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.
LGTM
Feel free to merge this now. I'm probably not finishing up the release until Monday |
This PR is an alternative to #561, where the query handler method is not split into two. This approach uses an "enum" extractor to differentiate when handling subgraph or deployment queries.
QuerySelector
extractorselector
field within the extractor