From fcaffcbe49e542c66a38ba2567b8256efc9f336e Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Thu, 29 Feb 2024 20:56:48 -0500 Subject: [PATCH] Fix martin `--auto-bounds` silently ignored with `--config` (#1223) When starting martin with both `--auto-bounds` and `--config`, the auto-bounds param was silently ignored, but now it will notify of the override. Also includes a few minor formatting changes --- Cargo.lock | 1 + martin/Cargo.toml | 5 +++-- martin/src/args/pg.rs | 32 +++++++++++++++++++++----------- martin/src/args/srv.rs | 4 ++-- 4 files changed, 27 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ab1e5062..f042f3f2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2106,6 +2106,7 @@ dependencies = [ "criterion", "ctor", "deadpool-postgres", + "enum-display", "env_logger", "flate2", "futures", diff --git a/martin/Cargo.toml b/martin/Cargo.toml index 2f02e2c2..874f95d4 100644 --- a/martin/Cargo.toml +++ b/martin/Cargo.toml @@ -79,11 +79,13 @@ bit-set = { workspace = true, optional = true } brotli.workspace = true clap.workspace = true deadpool-postgres = { workspace = true, optional = true } +enum-display.workspace = true env_logger.workspace = true flate2.workspace = true futures.workspace = true itertools.workspace = true json-patch = { workspace = true, optional = true } +lambda-web = { workspace = true, optional = true } log.workspace = true martin-tile-utils.workspace = true mbtiles = { workspace = true, optional = true } @@ -92,8 +94,8 @@ num_cpus.workspace = true pbf_font_tools = { workspace = true, optional = true } pmtiles = { workspace = true, optional = true } postgis = { workspace = true, optional = true } -postgres-protocol = { workspace = true, optional = true } postgres = { workspace = true, optional = true } +postgres-protocol = { workspace = true, optional = true } regex.workspace = true rustls-native-certs.workspace = true rustls-pemfile.workspace = true @@ -110,7 +112,6 @@ tilejson.workspace = true tokio = { workspace = true, features = ["io-std"] } tokio-postgres-rustls = { workspace = true, optional = true } url.workspace = true -lambda-web = { workspace = true, optional = true } [dev-dependencies] cargo-husky.workspace = true diff --git a/martin/src/args/pg.rs b/martin/src/args/pg.rs index a0da3a00..26f0b895 100644 --- a/martin/src/args/pg.rs +++ b/martin/src/args/pg.rs @@ -1,6 +1,7 @@ use std::time::Duration; use clap::ValueEnum; +use enum_display::EnumDisplay; use log::{info, warn}; use serde::{Deserialize, Serialize}; @@ -13,8 +14,11 @@ use crate::utils::{OptBoolObj, OptOneMany}; // Must match the help string for BoundsType::Quick pub const DEFAULT_BOUNDS_TIMEOUT: Duration = Duration::from_secs(5); -#[derive(PartialEq, Eq, Default, Debug, Clone, Copy, Serialize, Deserialize, ValueEnum)] +#[derive( + PartialEq, Eq, Default, Debug, Clone, Copy, Serialize, Deserialize, ValueEnum, EnumDisplay, +)] #[serde(rename_all = "lowercase")] +#[enum_display(case = "Kebab")] pub enum BoundsCalcType { /// Compute table geometry bounds, but abort if it takes longer than 5 seconds. #[default] @@ -37,7 +41,7 @@ pub struct PgArgs { /// If a spatial PG table has SRID 0, then this default SRID will be used as a fallback. #[arg(short, long)] pub default_srid: Option, - #[arg(help = format!("Maximum Postgres connections pool size [DEFAULT: {}]", POOL_SIZE_DEFAULT), short, long)] + #[arg(help = format!("Maximum Postgres connections pool size [DEFAULT: {POOL_SIZE_DEFAULT}]"), short, long)] pub pool_size: Option, /// Limit the number of features in a tile from a PG table source. #[arg(short, long)] @@ -76,29 +80,35 @@ impl PgArgs { } } + /// Apply CLI parameters from `self` to the configuration loaded from the config file `pg_config` pub fn override_config<'a>(self, pg_config: &mut OptOneMany, env: &impl Env<'a>) { - if self.default_srid.is_some() { - info!("Overriding configured default SRID to {} on all Postgres connections because of a CLI parameter", self.default_srid.unwrap()); + if let Some(default_srid) = self.default_srid { + info!("Overriding configured default SRID to {default_srid} on all Postgres connections because of a CLI parameter"); pg_config.iter_mut().for_each(|c| { c.default_srid = self.default_srid; }); } - if self.pool_size.is_some() { - info!("Overriding configured pool size to {} on all Postgres connections because of a CLI parameter", self.pool_size.unwrap()); + if let Some(pool_size) = self.pool_size { + info!("Overriding configured pool size to {pool_size} on all Postgres connections because of a CLI parameter"); pg_config.iter_mut().for_each(|c| { c.pool_size = self.pool_size; }); } - if self.max_feature_count.is_some() { - info!("Overriding maximum feature count to {} on all Postgres connections because of a CLI parameter", self.max_feature_count.unwrap()); + if let Some(auto_bounds) = self.auto_bounds { + info!("Overriding auto_bounds to {auto_bounds} on all Postgres connections because of a CLI parameter"); + pg_config.iter_mut().for_each(|c| { + c.auto_bounds = self.auto_bounds; + }); + } + if let Some(max_feature_count) = self.max_feature_count { + info!("Overriding maximum feature count to {max_feature_count} on all Postgres connections because of a CLI parameter"); pg_config.iter_mut().for_each(|c| { c.max_feature_count = self.max_feature_count; }); } - - if self.ca_root_file.is_some() { + if let Some(ref ca_root_file) = self.ca_root_file { info!("Overriding root certificate file to {} on all Postgres connections because of a CLI parameter", - self.ca_root_file.as_ref().unwrap().display()); + ca_root_file.display()); pg_config.iter_mut().for_each(|c| { c.ssl_certificates.ssl_root_cert = self.ca_root_file.clone(); }); diff --git a/martin/src/args/srv.rs b/martin/src/args/srv.rs index a3eddb72..ac07e755 100644 --- a/martin/src/args/srv.rs +++ b/martin/src/args/srv.rs @@ -6,9 +6,9 @@ use crate::srv::{SrvConfig, KEEP_ALIVE_DEFAULT, LISTEN_ADDRESSES_DEFAULT}; #[derive(clap::Args, Debug, PartialEq, Default)] #[command(about, version)] pub struct SrvArgs { - #[arg(help = format!("Connection keep alive timeout. [DEFAULT: {}]", KEEP_ALIVE_DEFAULT), short, long)] + #[arg(help = format!("Connection keep alive timeout. [DEFAULT: {KEEP_ALIVE_DEFAULT}]"), short, long)] pub keep_alive: Option, - #[arg(help = format!("The socket address to bind. [DEFAULT: {}]", LISTEN_ADDRESSES_DEFAULT), short, long)] + #[arg(help = format!("The socket address to bind. [DEFAULT: {LISTEN_ADDRESSES_DEFAULT}]"), short, long)] pub listen_addresses: Option, /// Set TileJSON URL path prefix, ignoring X-Rewrite-URL header. Must begin with a `/`. Examples: `/`, `/tiles` #[arg(long)]