From 65a72c435a42440374404120fc615f773421ac8d Mon Sep 17 00:00:00 2001 From: zyy17 Date: Tue, 30 Apr 2024 17:06:11 +0800 Subject: [PATCH] refactor: add the main cli entrypoint struct `Command{}` to simplify `main()` --- src/cmd/src/bin/greptime.rs | 85 ++++++++++++++++++------------- src/cmd/src/cli.rs | 8 +-- src/cmd/src/datanode.rs | 36 ++++++------- src/cmd/src/frontend.rs | 29 ++++++----- src/cmd/src/lib.rs | 10 ---- src/cmd/src/metasrv.rs | 27 +++++----- src/cmd/src/options.rs | 25 ++++----- src/cmd/src/standalone.rs | 31 +++++------ tests-integration/src/database.rs | 4 +- 9 files changed, 130 insertions(+), 125 deletions(-) diff --git a/src/cmd/src/bin/greptime.rs b/src/cmd/src/bin/greptime.rs index 0bbfb6bc0407..77f43bfdb06e 100644 --- a/src/cmd/src/bin/greptime.rs +++ b/src/cmd/src/bin/greptime.rs @@ -16,24 +16,42 @@ use std::fmt; -use clap::{FromArgMatches, Parser, Subcommand}; +use clap::{Parser, Subcommand}; use cmd::error::Result; -use cmd::options::{CliOptions, Options}; -use cmd::{ - cli, datanode, frontend, greptimedb_cli, log_versions, metasrv, standalone, start_app, App, -}; +use cmd::options::{GlobalOptions, Options}; +use cmd::{cli, datanode, frontend, log_versions, metasrv, standalone, start_app, App}; use common_version::{short_version, version}; #[derive(Parser)] +#[command(name = "greptime", author, version, long_version = version!(), about)] +#[command(propagate_version = true)] +pub(crate) struct Command { + #[clap(subcommand)] + pub(crate) subcmd: SubCommand, + + #[clap(flatten)] + pub(crate) global_options: GlobalOptions, +} + +#[derive(Subcommand)] enum SubCommand { + /// Start datanode service. #[clap(name = "datanode")] Datanode(datanode::Command), + + /// Start frontend service. #[clap(name = "frontend")] Frontend(frontend::Command), + + /// Start metasrv service. #[clap(name = "metasrv")] Metasrv(metasrv::Command), + + /// Run greptimedb as a standalone service. #[clap(name = "standalone")] Standalone(standalone::Command), + + /// Execute the cli tools for greptimedb. #[clap(name = "cli")] Cli(cli::Command), } @@ -67,13 +85,13 @@ impl SubCommand { Ok(app) } - fn load_options(&self, cli_options: &CliOptions) -> Result { + fn load_options(&self, global_options: &GlobalOptions) -> Result { match self { - SubCommand::Datanode(cmd) => cmd.load_options(cli_options), - SubCommand::Frontend(cmd) => cmd.load_options(cli_options), - SubCommand::Metasrv(cmd) => cmd.load_options(cli_options), - SubCommand::Standalone(cmd) => cmd.load_options(cli_options), - SubCommand::Cli(cmd) => cmd.load_options(cli_options), + SubCommand::Datanode(cmd) => cmd.load_options(global_options), + SubCommand::Frontend(cmd) => cmd.load_options(global_options), + SubCommand::Metasrv(cmd) => cmd.load_options(global_options), + SubCommand::Standalone(cmd) => cmd.load_options(global_options), + SubCommand::Cli(cmd) => cmd.load_options(global_options), } } } @@ -96,44 +114,39 @@ static ALLOC: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc; #[tokio::main] async fn main() -> Result<()> { - let metadata = human_panic::Metadata { - version: env!("CARGO_PKG_VERSION").into(), - name: "GreptimeDB".into(), - authors: Default::default(), - homepage: "https://github.com/GreptimeTeam/greptimedb/discussions".into(), - }; - human_panic::setup_panic!(metadata); - - common_telemetry::set_panic_hook(); - - let version = version!(); - let cli = greptimedb_cli().version(version); - - let cli = SubCommand::augment_subcommands(cli); - - let args = cli.get_matches(); + setup_human_panic(); + start(Command::parse()).await +} - let subcmd = match SubCommand::from_arg_matches(&args) { - Ok(subcmd) => subcmd, - Err(e) => e.exit(), - }; +async fn start(cli: Command) -> Result<()> { + let subcmd = cli.subcmd; let app_name = subcmd.to_string(); - let cli_options = CliOptions::new(&args); - - let opts = subcmd.load_options(&cli_options)?; + let opts = subcmd.load_options(&cli.global_options)?; let _guard = common_telemetry::init_global_logging( &app_name, opts.logging_options(), - cli_options.tracing_options(), + cli.global_options.tracing_options(), opts.node_id(), ); - log_versions(version, short_version!()); + log_versions(version!(), short_version!()); let app = subcmd.build(opts).await?; start_app(app).await } + +fn setup_human_panic() { + let metadata = human_panic::Metadata { + version: env!("CARGO_PKG_VERSION").into(), + name: "GreptimeDB".into(), + authors: Default::default(), + homepage: "https://github.com/GreptimeTeam/greptimedb/discussions".into(), + }; + human_panic::setup_panic!(metadata); + + common_telemetry::set_panic_hook(); +} diff --git a/src/cmd/src/cli.rs b/src/cmd/src/cli.rs index c44b99ad6bf9..a5f089bfe705 100644 --- a/src/cmd/src/cli.rs +++ b/src/cmd/src/cli.rs @@ -36,7 +36,7 @@ use upgrade::UpgradeCommand; use self::export::ExportCommand; use crate::error::Result; -use crate::options::{CliOptions, Options}; +use crate::options::{GlobalOptions, Options}; use crate::App; #[async_trait] @@ -80,14 +80,14 @@ impl Command { self.cmd.build().await } - pub fn load_options(&self, cli_options: &CliOptions) -> Result { + pub fn load_options(&self, global_options: &GlobalOptions) -> Result { let mut logging_opts = LoggingOptions::default(); - if let Some(dir) = &cli_options.log_dir { + if let Some(dir) = &global_options.log_dir { logging_opts.dir.clone_from(dir); } - logging_opts.level.clone_from(&cli_options.log_level); + logging_opts.level.clone_from(&global_options.log_level); Ok(Options::Cli(Box::new(logging_opts))) } diff --git a/src/cmd/src/datanode.rs b/src/cmd/src/datanode.rs index 58167024b2c9..dba7c04a9e67 100644 --- a/src/cmd/src/datanode.rs +++ b/src/cmd/src/datanode.rs @@ -28,7 +28,7 @@ use servers::Mode; use snafu::{OptionExt, ResultExt}; use crate::error::{MissingConfigSnafu, Result, ShutdownDatanodeSnafu, StartDatanodeSnafu}; -use crate::options::{CliOptions, Options}; +use crate::options::{GlobalOptions, Options}; use crate::App; pub struct Instance { @@ -82,8 +82,8 @@ impl Command { self.subcmd.build(opts).await } - pub fn load_options(&self, cli_options: &CliOptions) -> Result { - self.subcmd.load_options(cli_options) + pub fn load_options(&self, global_options: &GlobalOptions) -> Result { + self.subcmd.load_options(global_options) } } @@ -99,9 +99,9 @@ impl SubCommand { } } - fn load_options(&self, cli_options: &CliOptions) -> Result { + fn load_options(&self, global_options: &GlobalOptions) -> Result { match self { - SubCommand::Start(cmd) => cmd.load_options(cli_options), + SubCommand::Start(cmd) => cmd.load_options(global_options), } } } @@ -131,19 +131,19 @@ struct StartCommand { } impl StartCommand { - fn load_options(&self, cli_options: &CliOptions) -> Result { + fn load_options(&self, global_options: &GlobalOptions) -> Result { let mut opts: DatanodeOptions = Options::load_layered_options( self.config_file.as_deref(), self.env_prefix.as_ref(), DatanodeOptions::env_list_keys(), )?; - if let Some(dir) = &cli_options.log_dir { + if let Some(dir) = &global_options.log_dir { opts.logging.dir.clone_from(dir); } - if cli_options.log_level.is_some() { - opts.logging.level.clone_from(&cli_options.log_level); + if global_options.log_level.is_some() { + opts.logging.level.clone_from(&global_options.log_level); } if let Some(addr) = &self.rpc_addr { @@ -259,7 +259,7 @@ mod tests { use servers::Mode; use super::*; - use crate::options::{CliOptions, ENV_VAR_SEP}; + use crate::options::{GlobalOptions, ENV_VAR_SEP}; #[test] fn test_read_from_config_file() { @@ -315,7 +315,8 @@ mod tests { ..Default::default() }; - let Options::Datanode(options) = cmd.load_options(&CliOptions::default()).unwrap() else { + let Options::Datanode(options) = cmd.load_options(&GlobalOptions::default()).unwrap() + else { unreachable!() }; @@ -377,7 +378,7 @@ mod tests { #[test] fn test_try_from_cmd() { if let Options::Datanode(opt) = StartCommand::default() - .load_options(&CliOptions::default()) + .load_options(&GlobalOptions::default()) .unwrap() { assert_eq!(Mode::Standalone, opt.mode) @@ -388,7 +389,7 @@ mod tests { metasrv_addr: Some(vec!["127.0.0.1:3002".to_string()]), ..Default::default() }) - .load_options(&CliOptions::default()) + .load_options(&GlobalOptions::default()) .unwrap() { assert_eq!(Mode::Distributed, opt.mode) @@ -398,7 +399,7 @@ mod tests { metasrv_addr: Some(vec!["127.0.0.1:3002".to_string()]), ..Default::default() }) - .load_options(&CliOptions::default()) + .load_options(&GlobalOptions::default()) .is_err()); // Providing node_id but leave metasrv_addr absent is ok since metasrv_addr has default value @@ -406,7 +407,7 @@ mod tests { node_id: Some(42), ..Default::default() }) - .load_options(&CliOptions::default()) + .load_options(&GlobalOptions::default()) .is_ok()); } @@ -415,7 +416,7 @@ mod tests { let cmd = StartCommand::default(); let options = cmd - .load_options(&CliOptions { + .load_options(&GlobalOptions { log_dir: Some("/tmp/greptimedb/test/logs".to_string()), log_level: Some("debug".to_string()), @@ -504,7 +505,8 @@ mod tests { ..Default::default() }; - let Options::Datanode(opts) = command.load_options(&CliOptions::default()).unwrap() + let Options::Datanode(opts) = + command.load_options(&GlobalOptions::default()).unwrap() else { unreachable!() }; diff --git a/src/cmd/src/frontend.rs b/src/cmd/src/frontend.rs index 021596cb3dc4..e38edc5a0a55 100644 --- a/src/cmd/src/frontend.rs +++ b/src/cmd/src/frontend.rs @@ -36,7 +36,7 @@ use servers::Mode; use snafu::{OptionExt, ResultExt}; use crate::error::{self, InitTimezoneSnafu, MissingConfigSnafu, Result, StartFrontendSnafu}; -use crate::options::{CliOptions, Options}; +use crate::options::{GlobalOptions, Options}; use crate::App; pub struct Instance { @@ -90,8 +90,8 @@ impl Command { self.subcmd.build(opts).await } - pub fn load_options(&self, cli_options: &CliOptions) -> Result { - self.subcmd.load_options(cli_options) + pub fn load_options(&self, global_options: &GlobalOptions) -> Result { + self.subcmd.load_options(global_options) } } @@ -107,9 +107,9 @@ impl SubCommand { } } - fn load_options(&self, cli_options: &CliOptions) -> Result { + fn load_options(&self, global_options: &GlobalOptions) -> Result { match self { - SubCommand::Start(cmd) => cmd.load_options(cli_options), + SubCommand::Start(cmd) => cmd.load_options(global_options), } } } @@ -147,19 +147,19 @@ pub struct StartCommand { } impl StartCommand { - fn load_options(&self, cli_options: &CliOptions) -> Result { + fn load_options(&self, global_options: &GlobalOptions) -> Result { let mut opts: FrontendOptions = Options::load_layered_options( self.config_file.as_deref(), self.env_prefix.as_ref(), FrontendOptions::env_list_keys(), )?; - if let Some(dir) = &cli_options.log_dir { + if let Some(dir) = &global_options.log_dir { opts.logging.dir.clone_from(dir); } - if cli_options.log_level.is_some() { - opts.logging.level.clone_from(&cli_options.log_level); + if global_options.log_level.is_some() { + opts.logging.level.clone_from(&global_options.log_level); } let tls_opts = TlsOption::new( @@ -304,7 +304,7 @@ mod tests { use servers::http::HttpOptions; use super::*; - use crate::options::{CliOptions, ENV_VAR_SEP}; + use crate::options::{GlobalOptions, ENV_VAR_SEP}; #[test] fn test_try_from_start_command() { @@ -317,7 +317,8 @@ mod tests { ..Default::default() }; - let Options::Frontend(opts) = command.load_options(&CliOptions::default()).unwrap() else { + let Options::Frontend(opts) = command.load_options(&GlobalOptions::default()).unwrap() + else { unreachable!() }; @@ -367,7 +368,7 @@ mod tests { ..Default::default() }; - let Options::Frontend(fe_opts) = command.load_options(&CliOptions::default()).unwrap() + let Options::Frontend(fe_opts) = command.load_options(&GlobalOptions::default()).unwrap() else { unreachable!() }; @@ -414,7 +415,7 @@ mod tests { }; let options = cmd - .load_options(&CliOptions { + .load_options(&GlobalOptions { log_dir: Some("/tmp/greptimedb/test/logs".to_string()), log_level: Some("debug".to_string()), @@ -500,7 +501,7 @@ mod tests { }; let Options::Frontend(fe_opts) = - command.load_options(&CliOptions::default()).unwrap() + command.load_options(&GlobalOptions::default()).unwrap() else { unreachable!() }; diff --git a/src/cmd/src/lib.rs b/src/cmd/src/lib.rs index 9b4ba43bd330..7a5aa44ff488 100644 --- a/src/cmd/src/lib.rs +++ b/src/cmd/src/lib.rs @@ -15,7 +15,6 @@ #![feature(assert_matches, let_chains)] use async_trait::async_trait; -use clap::arg; use common_telemetry::{error, info}; pub mod cli; @@ -79,15 +78,6 @@ pub fn log_versions(version_string: &str, app_version: &str) { log_env_flags(); } -pub fn greptimedb_cli() -> clap::Command { - let cmd = clap::Command::new("greptimedb").subcommand_required(true); - - #[cfg(feature = "tokio-console")] - let cmd = cmd.arg(arg!(--"tokio-console-addr"[TOKIO_CONSOLE_ADDR])); - - cmd.args([arg!(--"log-dir"[LOG_DIR]), arg!(--"log-level"[LOG_LEVEL])]) -} - fn log_env_flags() { info!("command line arguments"); for argument in std::env::args() { diff --git a/src/cmd/src/metasrv.rs b/src/cmd/src/metasrv.rs index d83f7a460e3e..482a5c1e0336 100644 --- a/src/cmd/src/metasrv.rs +++ b/src/cmd/src/metasrv.rs @@ -22,7 +22,7 @@ use meta_srv::metasrv::MetasrvOptions; use snafu::ResultExt; use crate::error::{self, Result, StartMetaServerSnafu}; -use crate::options::{CliOptions, Options}; +use crate::options::{GlobalOptions, Options}; use crate::App; pub struct Instance { @@ -68,8 +68,8 @@ impl Command { self.subcmd.build(opts).await } - pub fn load_options(&self, cli_options: &CliOptions) -> Result { - self.subcmd.load_options(cli_options) + pub fn load_options(&self, global_options: &GlobalOptions) -> Result { + self.subcmd.load_options(global_options) } } @@ -85,9 +85,9 @@ impl SubCommand { } } - fn load_options(&self, cli_options: &CliOptions) -> Result { + fn load_options(&self, global_options: &GlobalOptions) -> Result { match self { - SubCommand::Start(cmd) => cmd.load_options(cli_options), + SubCommand::Start(cmd) => cmd.load_options(global_options), } } } @@ -126,19 +126,19 @@ struct StartCommand { } impl StartCommand { - fn load_options(&self, cli_options: &CliOptions) -> Result { + fn load_options(&self, global_options: &GlobalOptions) -> Result { let mut opts: MetasrvOptions = Options::load_layered_options( self.config_file.as_deref(), self.env_prefix.as_ref(), MetasrvOptions::env_list_keys(), )?; - if let Some(dir) = &cli_options.log_dir { + if let Some(dir) = &global_options.log_dir { opts.logging.dir.clone_from(dir); } - if cli_options.log_level.is_some() { - opts.logging.level.clone_from(&cli_options.log_level); + if global_options.log_level.is_some() { + opts.logging.level.clone_from(&global_options.log_level); } if let Some(addr) = &self.bind_addr { @@ -235,7 +235,7 @@ mod tests { ..Default::default() }; - let Options::Metasrv(options) = cmd.load_options(&CliOptions::default()).unwrap() else { + let Options::Metasrv(options) = cmd.load_options(&GlobalOptions::default()).unwrap() else { unreachable!() }; assert_eq!("127.0.0.1:3002".to_string(), options.bind_addr); @@ -270,7 +270,7 @@ mod tests { ..Default::default() }; - let Options::Metasrv(options) = cmd.load_options(&CliOptions::default()).unwrap() else { + let Options::Metasrv(options) = cmd.load_options(&GlobalOptions::default()).unwrap() else { unreachable!() }; assert_eq!("127.0.0.1:3002".to_string(), options.bind_addr); @@ -315,7 +315,7 @@ mod tests { }; let options = cmd - .load_options(&CliOptions { + .load_options(&GlobalOptions { log_dir: Some("/tmp/greptimedb/test/logs".to_string()), log_level: Some("debug".to_string()), @@ -379,7 +379,8 @@ mod tests { ..Default::default() }; - let Options::Metasrv(opts) = command.load_options(&CliOptions::default()).unwrap() + let Options::Metasrv(opts) = + command.load_options(&GlobalOptions::default()).unwrap() else { unreachable!() }; diff --git a/src/cmd/src/options.rs b/src/cmd/src/options.rs index 8fd974b939dc..4614b2a7806f 100644 --- a/src/cmd/src/options.rs +++ b/src/cmd/src/options.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use clap::ArgMatches; +use clap::Parser; use common_config::KvBackendConfig; use common_telemetry::logging::{LoggingOptions, TracingOptions}; use common_wal::config::MetasrvWalConfig; @@ -61,26 +61,23 @@ pub enum Options { Cli(Box), } -#[derive(Default)] -pub struct CliOptions { +#[derive(Parser, Default, Debug, Clone)] +pub struct GlobalOptions { + #[clap(long, value_name = "LOG_DIR")] + #[arg(global = true)] pub log_dir: Option, + + #[clap(long, value_name = "LOG_LEVEL")] + #[arg(global = true)] pub log_level: Option, #[cfg(feature = "tokio-console")] + #[clap(long, value_name = "TOKIO_CONSOLE_ADDR")] + #[arg(global = true)] pub tokio_console_addr: Option, } -impl CliOptions { - pub fn new(args: &ArgMatches) -> Self { - Self { - log_dir: args.get_one::("log-dir").cloned(), - log_level: args.get_one::("log-level").cloned(), - - #[cfg(feature = "tokio-console")] - tokio_console_addr: args.get_one::("tokio-console-addr").cloned(), - } - } - +impl GlobalOptions { pub fn tracing_options(&self) -> TracingOptions { TracingOptions { #[cfg(feature = "tokio-console")] diff --git a/src/cmd/src/standalone.rs b/src/cmd/src/standalone.rs index b4317de115e5..e55e98afab46 100644 --- a/src/cmd/src/standalone.rs +++ b/src/cmd/src/standalone.rs @@ -61,7 +61,7 @@ use crate::error::{ Result, ShutdownDatanodeSnafu, ShutdownFrontendSnafu, StartDatanodeSnafu, StartFrontendSnafu, StartProcedureManagerSnafu, StartWalOptionsAllocatorSnafu, StopProcedureManagerSnafu, }; -use crate::options::{CliOptions, MixOptions, Options}; +use crate::options::{GlobalOptions, MixOptions, Options}; use crate::App; #[derive(Parser)] @@ -75,8 +75,8 @@ impl Command { self.subcmd.build(opts).await } - pub fn load_options(&self, cli_options: &CliOptions) -> Result { - self.subcmd.load_options(cli_options) + pub fn load_options(&self, global_options: &GlobalOptions) -> Result { + self.subcmd.load_options(global_options) } } @@ -92,9 +92,9 @@ impl SubCommand { } } - fn load_options(&self, cli_options: &CliOptions) -> Result { + fn load_options(&self, global_options: &GlobalOptions) -> Result { match self { - SubCommand::Start(cmd) => cmd.load_options(cli_options), + SubCommand::Start(cmd) => cmd.load_options(global_options), } } } @@ -276,29 +276,29 @@ pub struct StartCommand { } impl StartCommand { - fn load_options(&self, cli_options: &CliOptions) -> Result { + fn load_options(&self, global_options: &GlobalOptions) -> Result { let opts: StandaloneOptions = Options::load_layered_options( self.config_file.as_deref(), self.env_prefix.as_ref(), StandaloneOptions::env_list_keys(), )?; - self.convert_options(cli_options, opts) + self.convert_options(global_options, opts) } pub fn convert_options( &self, - cli_options: &CliOptions, + global_options: &GlobalOptions, mut opts: StandaloneOptions, ) -> Result { opts.mode = Mode::Standalone; - if let Some(dir) = &cli_options.log_dir { + if let Some(dir) = &global_options.log_dir { opts.logging.dir.clone_from(dir); } - if cli_options.log_level.is_some() { - opts.logging.level.clone_from(&cli_options.log_level); + if global_options.log_level.is_some() { + opts.logging.level.clone_from(&global_options.log_level); } let tls_opts = TlsOption::new( @@ -529,7 +529,7 @@ mod tests { use servers::Mode; use super::*; - use crate::options::{CliOptions, ENV_VAR_SEP}; + use crate::options::{GlobalOptions, ENV_VAR_SEP}; #[tokio::test] async fn test_try_from_start_command_to_anymap() { @@ -617,7 +617,8 @@ mod tests { ..Default::default() }; - let Options::Standalone(options) = cmd.load_options(&CliOptions::default()).unwrap() else { + let Options::Standalone(options) = cmd.load_options(&GlobalOptions::default()).unwrap() + else { unreachable!() }; let fe_opts = options.frontend; @@ -673,7 +674,7 @@ mod tests { }; let Options::Standalone(opts) = cmd - .load_options(&CliOptions { + .load_options(&GlobalOptions { log_dir: Some("/tmp/greptimedb/test/logs".to_string()), log_level: Some("debug".to_string()), @@ -746,7 +747,7 @@ mod tests { }; let Options::Standalone(opts) = - command.load_options(&CliOptions::default()).unwrap() + command.load_options(&GlobalOptions::default()).unwrap() else { unreachable!() }; diff --git a/tests-integration/src/database.rs b/tests-integration/src/database.rs index 5ad7c90686fe..dfe4164b2059 100644 --- a/tests-integration/src/database.rs +++ b/tests-integration/src/database.rs @@ -274,7 +274,7 @@ mod tests { use clap::Parser; use client::Client; use cmd::error::Result as CmdResult; - use cmd::options::{CliOptions, Options}; + use cmd::options::{GlobalOptions, Options}; use cmd::{cli, standalone, App}; use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME}; @@ -313,7 +313,7 @@ mod tests { &*output_dir.path().to_string_lossy(), ]); let Options::Standalone(standalone_opts) = - standalone.load_options(&CliOptions::default())? + standalone.load_options(&GlobalOptions::default())? else { unreachable!() };