Improve cli parsing (#284)

Author: matthewmturner

.github/workflows/test.yml

@@ -270,7 +270,7 @@ jobs:
         uses: ./.github/actions/setup-rust
       - name: Start FlightSQL Server
         run: |
-          cargo r --features=flightsql -- --serve --config data/configs/flightsql_basic.toml &
+          cargo r --features=flightsql -- serve-flight-sql --config data/configs/flightsql_basic.toml &
       - name: Run auth tests
         run: |
           cargo t --features=flightsql extension_cases::auth_basic
@@ -298,7 +298,7 @@ jobs:
         uses: ./.github/actions/setup-rust
       - name: Start FlightSQL Server
         run: |
-          cargo r --features=flightsql -- --serve --config data/configs/flightsql_bearer.toml &
+          cargo r --features=flightsql -- serve-flight-sql --config data/configs/flightsql_bearer.toml &
       - name: Run auth tests
         run: |
           cargo t --features=flightsql extension_cases::auth_bearer

README.md

@@ -156,7 +156,7 @@ This feature is still in it's early stages and is expected to evolve.  Once it h
 
 ## `dft` FlightSQL Server
 
-The `dft` FlightSQL server (feature flag `flightsql`) is a Flight service that can be used to execute SQL queries against DataFusion.  The server is started by running `dft --serve` and can optionally run your configured DDL with the `--run-ddl` parameter.  Prometheus metrics are automatically exported as part of this.
+The `dft` FlightSQL server (feature flag `flightsql`) is a Flight service that can be used to execute SQL queries against DataFusion.  The server is started by running `dft serve-flight-sql` and can optionally run your configured DDL with the `--run-ddl` parameter.  Prometheus metrics are automatically exported as part of this.
 
 This feature is experimental and does not currently implement all FlightSQL endpoints.  Endpoints will be added in tandem with adding more features to the FlightSQL clients within the TUI and CLI.
 

src/args.rs

@@ -18,7 +18,7 @@
 //! Command line argument parsing: [`DftArgs`]
 
 use crate::config::get_data_dir;
-use clap::Parser;
+use clap::{Parser, Subcommand};
 use std::path::{Path, PathBuf};
 
 const LONG_ABOUT: &str = "
@@ -67,10 +67,6 @@ pub struct DftArgs {
     #[clap(long, short, help = "Only show how long the query took to run")]
     pub time: bool,
 
-    #[cfg(feature = "flightsql")]
-    #[clap(long, help = "Start a FlightSQL server")]
-    pub serve: bool,
-
     #[clap(long, short, help = "Benchmark the provided query")]
     pub bench: bool,
 
@@ -102,10 +98,16 @@ pub struct DftArgs {
         help = "Path to save output to. Type is inferred from file suffix"
     )]
     pub output: Option<PathBuf>,
+
+    #[command(subcommand)]
+    pub command: Option<Command>,
 }
 
 impl DftArgs {
     pub fn config_path(&self) -> PathBuf {
+        if let Some(Command::ServeFlightSql { config: Some(cfg) }) = &self.command {
+            return Path::new(cfg).to_path_buf();
+        }
         if let Some(config) = self.config.as_ref() {
             Path::new(config).to_path_buf()
         } else {
@@ -116,6 +118,15 @@ impl DftArgs {
     }
 }
 
+#[derive(Clone, Debug, Subcommand)]
+pub enum Command {
+    /// Start a FlightSQL server
+    ServeFlightSql {
+        #[clap(short, long)]
+        config: Option<String>,
+    },
+}
+
 fn parse_valid_file(file: &str) -> std::result::Result<PathBuf, String> {
     let path = PathBuf::from(file);
     if !path.exists() {

src/main.rs

@@ -19,6 +19,7 @@ use clap::Parser;
 use color_eyre::Result;
 use datafusion_app::local::ExecutionContext;
 use datafusion_app::{config::merge_configs, extensions::DftSessionStateBuilder};
+use datafusion_dft::args::Command;
 use datafusion_dft::{
     args::DftArgs,
     cli::CliApp,
@@ -52,7 +53,7 @@ fn main() -> Result<()> {
 
 fn should_init_env_logger(cli: &DftArgs) -> bool {
     #[cfg(feature = "flightsql")]
-    if cli.serve {
+    if let Some(Command::ServeFlightSql { .. }) = cli.command {
         return true;
     }
     if !cli.files.is_empty() || !cli.commands.is_empty() {
@@ -67,7 +68,7 @@ async fn app_entry_point(cli: DftArgs) -> Result<()> {
     }
     let state = state::initialize(cli.config_path());
     #[cfg(feature = "flightsql")]
-    if cli.serve {
+    if let Some(Command::ServeFlightSql { .. }) = cli.command {
         let merged_exec_config = merge_configs(
             state.config.shared.clone(),
             state.config.flightsql_server.execution.clone(),