Skip to content
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

Properly implement subgraph client fallback from local deployment to remote URL #81

Merged
merged 6 commits into from
Oct 25, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions common/src/allocations/monitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ pub fn indexer_allocations(

#[cfg(test)]
mod test {
use reqwest::Url;
use serde_json::json;
use wiremock::{
matchers::{body_string_contains, method, path},
Expand All @@ -202,17 +203,14 @@ mod test {
async fn setup_mock_network_subgraph() -> (&'static SubgraphClient, MockServer) {
// Set up a mock network subgraph
let mock_server = MockServer::start().await;
let network_subgraph_endpoint = SubgraphClient::local_deployment_endpoint(
let network_subgraph_endpoint = Url::parse(&format!(
"{}/subgraphs/id/{}",
&mock_server.uri(),
&test_vectors::NETWORK_SUBGRAPH_DEPLOYMENT,
)
.unwrap();
let network_subgraph = SubgraphClient::new(
Some(&mock_server.uri()),
Some(&test_vectors::NETWORK_SUBGRAPH_DEPLOYMENT),
network_subgraph_endpoint.as_ref(),
)
*test_vectors::NETWORK_SUBGRAPH_DEPLOYMENT
))
.unwrap();
let network_subgraph =
SubgraphClient::new("network-subgraph", network_subgraph_endpoint.as_ref()).unwrap();

// Mock result for current epoch requests
mock_server
Expand Down
16 changes: 7 additions & 9 deletions common/src/attestations/dispute_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ pub fn dispute_manager(

#[cfg(test)]
mod test {
use reqwest::Url;
use serde_json::json;
use wiremock::{
matchers::{method, path},
Expand All @@ -97,17 +98,14 @@ mod test {
async fn setup_mock_network_subgraph() -> (&'static SubgraphClient, MockServer) {
// Set up a mock network subgraph
let mock_server = MockServer::start().await;
let network_subgraph_endpoint = SubgraphClient::local_deployment_endpoint(
let network_subgraph_endpoint = Url::parse(&format!(
"{}/subgraphs/id/{}",
&mock_server.uri(),
&test_vectors::NETWORK_SUBGRAPH_DEPLOYMENT,
)
.unwrap();
let network_subgraph = SubgraphClient::new(
Some(&mock_server.uri()),
Some(&test_vectors::NETWORK_SUBGRAPH_DEPLOYMENT),
network_subgraph_endpoint.as_ref(),
)
*test_vectors::NETWORK_SUBGRAPH_DEPLOYMENT
))
.unwrap();
let network_subgraph =
SubgraphClient::new("network-subgraph", network_subgraph_endpoint.as_ref()).unwrap();

// Mock result for current epoch requests
mock_server
Expand Down
15 changes: 6 additions & 9 deletions common/src/escrow_accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ pub fn escrow_accounts(

#[cfg(test)]
mod tests {
use reqwest::Url;
use wiremock::matchers::{method, path};
use wiremock::{Mock, MockServer, ResponseTemplate};

Expand All @@ -126,18 +127,14 @@ mod tests {
async fn test_current_accounts() {
// Set up a mock escrow subgraph
let mock_server = MockServer::start().await;
let escrow_subgraph_endpoint = SubgraphClient::local_deployment_endpoint(
let escrow_subgraph_endpoint = Url::parse(&format!(
"{}/subgraphs/id/{}",
&mock_server.uri(),
&test_vectors::ESCROW_SUBGRAPH_DEPLOYMENT,
)
*test_vectors::ESCROW_SUBGRAPH_DEPLOYMENT
))
.unwrap();
let escrow_subgraph = Box::leak(Box::new(
SubgraphClient::new(
Some(&mock_server.uri()),
Some(&test_vectors::ESCROW_SUBGRAPH_DEPLOYMENT),
escrow_subgraph_endpoint.as_ref(),
)
.unwrap(),
SubgraphClient::new("escrow-subgraph", escrow_subgraph_endpoint.as_ref()).unwrap(),
));

let mock = Mock::given(method("POST"))
Expand Down
56 changes: 17 additions & 39 deletions common/src/subgraph_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use graphql::http::Response;
use reqwest::{header, Client, Url};
use serde::de::Deserialize;
use serde_json::Value;
use toolshed::thegraph::DeploymentId;

/// Network subgraph query wrapper
///
Expand All @@ -20,48 +19,32 @@ pub struct SubgraphClient {
}

impl SubgraphClient {
pub fn new(
graph_node_query_endpoint: Option<&str>,
deployment: Option<&DeploymentId>,
subgraph_url: &str,
) -> Result<Self, anyhow::Error> {
// TODO: Check indexing status of the local subgraph deployment
// if the deployment is healthy and synced, use local_subgraoh_endpoint
let _local_subgraph_endpoint = match (graph_node_query_endpoint, deployment) {
(Some(endpoint), Some(id)) => Some(Self::local_deployment_endpoint(endpoint, id)?),
_ => None,
};

let subgraph_url = Url::parse(subgraph_url)
.map_err(|e| anyhow!("Could not parse subgraph url `{}`: {}", subgraph_url, e))?;
pub fn new(name: &str, query_url: &str) -> Result<Self, anyhow::Error> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need name? It seems to be only used for error messages.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, no, let's not include it.

let query_url = Url::parse(query_url).map_err(|e| {
anyhow!(
"Could not parse `{}` subgraph query URL `{}`: {}",
name,
query_url,
e
)
})?;

let client = reqwest::Client::builder()
.user_agent("indexer-common")
.build()
.expect("Could not build a client for the Graph Node query endpoint");
.map_err(|err| {
anyhow!(
"Could not build a client for `{name}` subgraph query URL `{query_url}`: {err}"
)
})
.expect("Building subgraph client");

Ok(Self {
client,
subgraph_url: Arc::new(subgraph_url),
subgraph_url: Arc::new(query_url),
})
}

pub fn local_deployment_endpoint(
graph_node_query_endpoint: &str,
deployment: &DeploymentId,
) -> Result<Url, anyhow::Error> {
Url::parse(graph_node_query_endpoint)
.and_then(|u| u.join("/subgraphs/id/"))
.and_then(|u| u.join(&deployment.to_string()))
.map_err(|e| {
anyhow!(
"Could not parse Graph Node query endpoint for subgraph deployment `{}`: {}",
deployment,
e
)
})
}

pub async fn query<T: for<'de> Deserialize<'de>>(
&self,
body: &Value,
Expand Down Expand Up @@ -117,12 +100,7 @@ mod test {
}

fn network_subgraph_client() -> SubgraphClient {
SubgraphClient::new(
Some(GRAPH_NODE_STATUS_ENDPOINT),
Some(&test_vectors::NETWORK_SUBGRAPH_DEPLOYMENT),
NETWORK_SUBGRAPH_URL,
)
.unwrap()
SubgraphClient::new("network-subgraph", NETWORK_SUBGRAPH_URL).unwrap()
}

#[tokio::test]
Expand Down
6 changes: 2 additions & 4 deletions service/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,7 @@ async fn main() -> Result<(), std::io::Error> {
// no problem.
let network_subgraph = Box::leak(Box::new(
SubgraphClient::new(
Some(&config.indexer_infrastructure.graph_node_query_endpoint),
config.network_subgraph.network_subgraph_deployment.as_ref(),
"network-subgraph",
&config.network_subgraph.network_subgraph_endpoint,
)
.expect("Failed to set up network subgraph client"),
Expand Down Expand Up @@ -108,8 +107,7 @@ async fn main() -> Result<(), std::io::Error> {

let escrow_subgraph = Box::leak(Box::new(
SubgraphClient::new(
Some(&config.indexer_infrastructure.graph_node_query_endpoint),
config.escrow_subgraph.escrow_subgraph_deployment.as_ref(),
"escrow-subgraph",
&config.escrow_subgraph.escrow_subgraph_endpoint,
)
.expect("Failed to set up escrow subgraph client"),
Expand Down