From 47f6c026a85e8c48e46c681c69e5eafb46e7bb19 Mon Sep 17 00:00:00 2001 From: Ashok Menon Date: Wed, 2 Oct 2024 21:10:23 +0100 Subject: [PATCH] chore(graphql): deduplicate configs and clap args ## 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. --- crates/sui-cluster-test/src/cluster.rs | 14 +++-- crates/sui-graphql-rpc/src/commands.rs | 33 ++++-------- crates/sui-graphql-rpc/src/config.rs | 51 +++++++------------ crates/sui-graphql-rpc/src/main.rs | 21 +++----- crates/sui-graphql-rpc/src/server/builder.rs | 6 +-- .../sui-graphql-rpc/src/test_infra/cluster.rs | 4 +- crates/sui/src/sui_commands.rs | 15 +++--- 7 files changed, 52 insertions(+), 92 deletions(-) diff --git a/crates/sui-cluster-test/src/cluster.rs b/crates/sui-cluster-test/src/cluster.rs index b90c8263ce4da..e0263658afae5 100644 --- a/crates/sui-cluster-test/src/cluster.rs +++ b/crates/sui-cluster-test/src/cluster.rs @@ -251,14 +251,12 @@ impl Cluster for LocalNewCluster { // Start the graphql service let graphql_address = graphql_address.parse::()?; - 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(), diff --git a/crates/sui-graphql-rpc/src/commands.rs b/crates/sui-graphql-rpc/src/commands.rs index 4b0eca46c11cb..b71a38de99b83 100644 --- a/crates/sui-graphql-rpc/src/commands.rs +++ b/crates/sui-graphql-rpc/src/commands.rs @@ -4,6 +4,8 @@ use clap::*; use std::path::PathBuf; +use crate::config::{ConnectionConfig, Ide, TxExecFullNodeConfig}; + #[derive(Parser)] #[clap( name = "sui-graphql-rpc", @@ -21,34 +23,17 @@ pub enum Command { }, StartServer { - /// The title to display at the top of the page - #[clap(short, long)] - ide_title: Option, - /// DB URL for data fetching - #[clap(short, long)] - db_url: Option, - /// Pool size for DB connections - #[clap(long)] - db_pool_size: Option, - /// Port to bind the server to - #[clap(short, long)] - port: Option, - /// Host to bind the server to - #[clap(long)] - host: Option, - /// Port to bind the prom server to - #[clap(long)] - prom_port: Option, - /// Host to bind the prom server to - #[clap(long)] - prom_host: Option, + #[clap(flatten)] + ide: Ide, + + #[clap(flatten)] + connection: ConnectionConfig, /// Path to TOML file containing configuration for service. #[clap(short, long)] config: Option, - /// RPC url to the Node for tx execution - #[clap(long)] - node_rpc_url: Option, + #[clap(flatten)] + tx_exec_full_node: TxExecFullNodeConfig, }, } diff --git a/crates/sui-graphql-rpc/src/config.rs b/crates/sui-graphql-rpc/src/config.rs index f2d446b91c510..1fb38b6be1bce 100644 --- a/crates/sui-graphql-rpc/src/config.rs +++ b/crates/sui-graphql-rpc/src/config.rs @@ -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, } @@ -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] @@ -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, } @@ -337,25 +351,6 @@ impl TxExecFullNodeConfig { } impl ConnectionConfig { - pub fn new( - port: Option, - host: Option, - db_url: Option, - db_pool_size: Option, - prom_url: Option, - prom_port: Option, - ) -> 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() } @@ -432,14 +427,6 @@ impl Limits { } } -impl Ide { - pub fn new(ide_title: Option) -> Self { - ide_title - .map(|ide_title| Ide { ide_title }) - .unwrap_or_default() - } -} - impl BackgroundTasksConfig { pub fn test_defaults() -> Self { Self { @@ -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, } } diff --git a/crates/sui-graphql-rpc/src/main.rs b/crates/sui-graphql-rpc/src/main.rs index 7dde900b1d86c..9a88a0908fd7a 100644 --- a/crates/sui-graphql-rpc/src/main.rs +++ b/crates/sui-graphql-rpc/src/main.rs @@ -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; @@ -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() @@ -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() }; diff --git a/crates/sui-graphql-rpc/src/server/builder.rs b/crates/sui-graphql-rpc/src/server/builder.rs index 4d2b35ac28f9b..e672715f91eaa 100644 --- a/crates/sui-graphql-rpc/src/server/builder.rs +++ b/crates/sui-graphql-rpc/src/server/builder.rs @@ -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 )) })?; @@ -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(); diff --git a/crates/sui-graphql-rpc/src/test_infra/cluster.rs b/crates/sui-graphql-rpc/src/test_infra/cluster.rs index 64f4f74c2ed42..9179f1f56aa72 100644 --- a/crates/sui-graphql-rpc/src/test_infra/cluster.rs +++ b/crates/sui-graphql-rpc/src/test_infra/cluster.rs @@ -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(); @@ -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(); diff --git a/crates/sui/src/sui_commands.rs b/crates/sui/src/sui_commands.rs index c08a72619bf97..b5e5bb1000f13 100644 --- a/crates/sui/src/sui_commands.rs +++ b/crates/sui/src/sui_commands.rs @@ -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()),