Skip to content

Commit

Permalink
chore(graphql): deduplicate configs and clap args
Browse files Browse the repository at this point in the history
## Description

Avoid duplicating fields for configs that are accepted as flags from the
command-line and can also be read from TOML file. Use the same struct
for both purposes but deocrate it with both `clap` and `serde` (or
`GraphQLConfig`) annotations so that it can serve both purposes.

## Test plan

This change should preserve the config/command-line interface -- checked
by running the GraphQL service with the same invocation, referencing all
flags, before and after the change.
  • Loading branch information
amnn committed Oct 3, 2024
1 parent bc528ae commit 47f6c02
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 92 deletions.
14 changes: 6 additions & 8 deletions crates/sui-cluster-test/src/cluster.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,14 +251,12 @@ impl Cluster for LocalNewCluster {

// Start the graphql service
let graphql_address = graphql_address.parse::<SocketAddr>()?;
let graphql_connection_config = ConnectionConfig::new(
Some(graphql_address.port()),
Some(graphql_address.ip().to_string()),
Some(pg_address),
None,
None,
None,
);
let graphql_connection_config = ConnectionConfig {
port: graphql_address.port(),
host: graphql_address.ip().to_string(),
db_url: pg_address,
..Default::default()
};

start_graphql_server_with_fn_rpc(
graphql_connection_config.clone(),
Expand Down
33 changes: 9 additions & 24 deletions crates/sui-graphql-rpc/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
use clap::*;
use std::path::PathBuf;

use crate::config::{ConnectionConfig, Ide, TxExecFullNodeConfig};

#[derive(Parser)]
#[clap(
name = "sui-graphql-rpc",
Expand All @@ -21,34 +23,17 @@ pub enum Command {
},

StartServer {
/// The title to display at the top of the page
#[clap(short, long)]
ide_title: Option<String>,
/// DB URL for data fetching
#[clap(short, long)]
db_url: Option<String>,
/// Pool size for DB connections
#[clap(long)]
db_pool_size: Option<u32>,
/// Port to bind the server to
#[clap(short, long)]
port: Option<u16>,
/// Host to bind the server to
#[clap(long)]
host: Option<String>,
/// Port to bind the prom server to
#[clap(long)]
prom_port: Option<u16>,
/// Host to bind the prom server to
#[clap(long)]
prom_host: Option<String>,
#[clap(flatten)]
ide: Ide,

#[clap(flatten)]
connection: ConnectionConfig,

/// Path to TOML file containing configuration for service.
#[clap(short, long)]
config: Option<PathBuf>,

/// RPC url to the Node for tx execution
#[clap(long)]
node_rpc_url: Option<String>,
#[clap(flatten)]
tx_exec_full_node: TxExecFullNodeConfig,
},
}
51 changes: 19 additions & 32 deletions crates/sui-graphql-rpc/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,25 @@ pub struct ServerConfig {
/// specific connections between this service and other services, and might differ from instance to
/// instance of the GraphQL service.
#[GraphQLConfig]
#[derive(Clone, Eq, PartialEq)]
#[derive(clap::Args, Clone, Eq, PartialEq)]
pub struct ConnectionConfig {
/// Port to bind the server to
#[clap(short, long, default_value_t = ConnectionConfig::default().port)]
pub port: u16,
/// Host to bind the server to
#[clap(long, default_value_t = ConnectionConfig::default().host)]
pub host: String,
/// DB URL for data fetching
#[clap(short, long, default_value_t = ConnectionConfig::default().db_url)]
pub db_url: String,
/// Pool size for DB connections
#[clap(long, default_value_t = ConnectionConfig::default().db_pool_size)]
pub db_pool_size: u32,
pub prom_url: String,
/// Host to bind the prom server to
#[clap(long, default_value_t = ConnectionConfig::default().prom_host)]
pub prom_host: String,
/// Port to bind the prom server to
#[clap(long, default_value_t = ConnectionConfig::default().prom_port)]
pub prom_port: u16,
}

Expand Down Expand Up @@ -172,8 +182,11 @@ impl Version {
}

#[GraphQLConfig]
#[derive(clap::Args)]
pub struct Ide {
pub(crate) ide_title: String,
/// The title to display at the top of the web-based GraphiQL IDE.
#[clap(short, long, default_value_t = Ide::default().ide_title)]
pub ide_title: String,
}

#[GraphQLConfig]
Expand All @@ -199,8 +212,9 @@ pub struct InternalFeatureConfig {
}

#[GraphQLConfig]
#[derive(Default)]
#[derive(clap::Args, Default)]
pub struct TxExecFullNodeConfig {
/// RPC URL for the fullnode to send transactions to execute and dry-run.
pub(crate) node_rpc_url: Option<String>,
}

Expand Down Expand Up @@ -337,25 +351,6 @@ impl TxExecFullNodeConfig {
}

impl ConnectionConfig {
pub fn new(
port: Option<u16>,
host: Option<String>,
db_url: Option<String>,
db_pool_size: Option<u32>,
prom_url: Option<String>,
prom_port: Option<u16>,
) -> Self {
let default = Self::default();
Self {
port: port.unwrap_or(default.port),
host: host.unwrap_or(default.host),
db_url: db_url.unwrap_or(default.db_url),
db_pool_size: db_pool_size.unwrap_or(default.db_pool_size),
prom_url: prom_url.unwrap_or(default.prom_url),
prom_port: prom_port.unwrap_or(default.prom_port),
}
}

pub fn db_name(&self) -> String {
self.db_url.split('/').last().unwrap().to_string()
}
Expand Down Expand Up @@ -432,14 +427,6 @@ impl Limits {
}
}

impl Ide {
pub fn new(ide_title: Option<String>) -> Self {
ide_title
.map(|ide_title| Ide { ide_title })
.unwrap_or_default()
}
}

impl BackgroundTasksConfig {
pub fn test_defaults() -> Self {
Self {
Expand Down Expand Up @@ -481,7 +468,7 @@ impl Default for ConnectionConfig {
host: "127.0.0.1".to_string(),
db_url: "postgres://postgres:postgrespw@localhost:5432/sui_indexer".to_string(),
db_pool_size: 10,
prom_url: "0.0.0.0".to_string(),
prom_host: "0.0.0.0".to_string(),
prom_port: 9184,
}
}
Expand Down
21 changes: 6 additions & 15 deletions crates/sui-graphql-rpc/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ use std::path::PathBuf;

use clap::Parser;
use sui_graphql_rpc::commands::Command;
use sui_graphql_rpc::config::{
ConnectionConfig, Ide, ServerConfig, ServiceConfig, TxExecFullNodeConfig, Version,
};
use sui_graphql_rpc::config::{ServerConfig, ServiceConfig, Version};
use sui_graphql_rpc::server::graphiql_server::start_graphiql_server;
use tokio_util::sync::CancellationToken;
use tokio_util::task::TaskTracker;
Expand Down Expand Up @@ -51,18 +49,11 @@ async fn main() {
}

Command::StartServer {
ide_title,
db_url,
db_pool_size,
port,
host,
ide,
connection,
config,
node_rpc_url,
prom_host,
prom_port,
tx_exec_full_node,
} => {
let connection =
ConnectionConfig::new(port, host, db_url, db_pool_size, prom_host, prom_port);
let service_config = service_config(config);
let _guard = telemetry_subscribers::TelemetryConfig::new()
.with_env()
Expand All @@ -74,8 +65,8 @@ async fn main() {
let server_config = ServerConfig {
connection,
service: service_config,
ide: Ide::new(ide_title),
tx_exec_full_node: TxExecFullNodeConfig::new(node_rpc_url),
ide,
tx_exec_full_node,
..ServerConfig::default()
};

Expand Down
6 changes: 3 additions & 3 deletions crates/sui-graphql-rpc/src/server/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,13 +378,13 @@ impl ServerBuilder {
// PROMETHEUS
let prom_addr: SocketAddr = format!(
"{}:{}",
config.connection.prom_url, config.connection.prom_port
config.connection.prom_host, config.connection.prom_port
)
.parse()
.map_err(|_| {
Error::Internal(format!(
"Failed to parse url {}, port {} into socket address",
config.connection.prom_url, config.connection.prom_port
config.connection.prom_host, config.connection.prom_port
))
})?;

Expand Down Expand Up @@ -706,7 +706,7 @@ pub mod tests {
host: "127.0.0.1".to_owned(),
db_url,
db_pool_size: 5,
prom_url: "127.0.0.1".to_owned(),
prom_host: "127.0.0.1".to_owned(),
prom_port: get_available_port(),
};
let service_config = service_config.unwrap_or_default();
Expand Down
4 changes: 2 additions & 2 deletions crates/sui-graphql-rpc/src/test_infra/cluster.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ pub async fn start_network_cluster() -> NetworkCluster {
host: "127.0.0.1".to_owned(),
db_url: database.database().url().as_str().to_owned(),
db_pool_size: 5,
prom_url: "127.0.0.1".to_owned(),
prom_host: "127.0.0.1".to_owned(),
prom_port: get_available_port(),
};
let data_ingestion_path = tempfile::tempdir().unwrap();
Expand Down Expand Up @@ -160,7 +160,7 @@ pub async fn serve_executor(
host: "127.0.0.1".to_owned(),
db_url: database.database().url().as_str().to_owned(),
db_pool_size: 5,
prom_url: "127.0.0.1".to_owned(),
prom_host: "127.0.0.1".to_owned(),
prom_port: get_available_port(),
};
let db_url = graphql_connection_config.db_url.clone();
Expand Down
15 changes: 7 additions & 8 deletions crates/sui/src/sui_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -727,14 +727,13 @@ async fn start(
let graphql_address = parse_host_port(input, DEFAULT_GRAPHQL_PORT)
.map_err(|_| anyhow!("Invalid graphql host and port"))?;
tracing::info!("Starting the GraphQL service at {graphql_address}");
let graphql_connection_config = ConnectionConfig::new(
Some(graphql_address.port()),
Some(graphql_address.ip().to_string()),
Some(pg_address),
None,
None,
None,
);
let graphql_connection_config = ConnectionConfig {
port: graphql_address.port(),
host: graphql_address.ip().to_string(),
db_url: pg_address,
..Default::default()
};

start_graphql_server_with_fn_rpc(
graphql_connection_config,
Some(fullnode_url.clone()),
Expand Down

0 comments on commit 47f6c02

Please sign in to comment.