use clap instead

Signed-off-by: Alex Chi <iskyzh@gmail.com>
This commit is contained in:
Alex Chi
2023-05-26 10:17:05 -04:00
parent 9073db2622
commit 5e92758d20
4 changed files with 46 additions and 129 deletions

1
Cargo.lock generated
View File

@@ -2628,6 +2628,7 @@ dependencies = [
"anyhow",
"byteorder",
"bytes",
"clap 4.3.0",
"const_format",
"enum-map",
"postgres_ffi",

View File

@@ -312,63 +312,8 @@ impl PageServerNode {
new_tenant_id: Option<TenantId>,
settings: HashMap<&str, &str>,
) -> anyhow::Result<TenantId> {
let mut settings = settings.clone();
let config = models::TenantConfig {
checkpoint_distance: settings
.remove("checkpoint_distance")
.map(|x| x.parse::<u64>())
.transpose()?,
checkpoint_timeout: settings.remove("checkpoint_timeout").map(|x| x.to_string()),
compaction_target_size: settings
.remove("compaction_target_size")
.map(|x| x.parse::<u64>())
.transpose()?,
compaction_period: settings.remove("compaction_period").map(|x| x.to_string()),
compaction_threshold: settings
.remove("compaction_threshold")
.map(|x| x.parse::<usize>())
.transpose()?,
gc_horizon: settings
.remove("gc_horizon")
.map(|x| x.parse::<u64>())
.transpose()?,
gc_period: settings.remove("gc_period").map(|x| x.to_string()),
image_creation_threshold: settings
.remove("image_creation_threshold")
.map(|x| x.parse::<usize>())
.transpose()?,
pitr_interval: settings.remove("pitr_interval").map(|x| x.to_string()),
walreceiver_connect_timeout: settings
.remove("walreceiver_connect_timeout")
.map(|x| x.to_string()),
lagging_wal_timeout: settings
.remove("lagging_wal_timeout")
.map(|x| x.to_string()),
max_lsn_wal_lag: settings
.remove("max_lsn_wal_lag")
.map(|x| x.parse::<NonZeroU64>())
.transpose()
.context("Failed to parse 'max_lsn_wal_lag' as non zero integer")?,
trace_read_requests: settings
.remove("trace_read_requests")
.map(|x| x.parse::<bool>())
.transpose()
.context("Failed to parse 'trace_read_requests' as bool")?,
eviction_policy: settings
.remove("eviction_policy")
.map(serde_json::from_str)
.transpose()
.context("Failed to parse 'eviction_policy' json")?,
min_resident_size_override: settings
.remove("min_resident_size_override")
.map(|x| x.parse::<u64>())
.transpose()
.context("Failed to parse 'min_resident_size_override' as integer")?,
evictions_low_residence_duration_metric_threshold: settings
.remove("evictions_low_residence_duration_metric_threshold")
.map(|x| x.to_string()),
};
let settings = settings.clone();
let config = models::TenantConfig::deserialize_from_settings(settings)?;
// If tenant ID was not specified, generate one
let new_tenant_id = new_tenant_id.unwrap_or(TenantId::generate());
@@ -398,7 +343,6 @@ impl PageServerNode {
tenant_id: TenantId,
settings: HashMap<&str, &str>,
) -> anyhow::Result<()> {
println!("{:#?}", settings);
let config = models::TenantConfig::deserialize_from_settings(settings)?;
self.http_request(Method::PUT, format!("{}/tenant/config", self.http_base_url))?
.json(&models::TenantConfigRequest { tenant_id, config })

View File

@@ -5,6 +5,7 @@ edition.workspace = true
license.workspace = true
[dependencies]
clap.workspace = true
serde.workspace = true
serde_with.workspace = true
serde_json.workspace = true

View File

@@ -1,13 +1,12 @@
use std::{
collections::HashMap,
fmt::Display,
num::{NonZeroU64, NonZeroUsize},
str::FromStr,
time::SystemTime,
};
use byteorder::{BigEndian, ReadBytesExt};
use serde::{de::value::MapDeserializer, Deserialize, Deserializer, Serialize};
use clap::Parser;
use serde::{Deserialize, Serialize};
use serde_with::{serde_as, DisplayFromStr};
use strum_macros;
use utils::{
@@ -149,83 +148,63 @@ impl std::ops::Deref for TenantCreateRequest {
}
}
pub fn deserialize_option_number_from_string<'de, T, D>(
deserializer: D,
) -> Result<Option<T>, D::Error>
where
D: Deserializer<'de>,
T: FromStr + serde::Deserialize<'de>,
<T as FromStr>::Err: Display,
{
#[derive(Deserialize)]
#[serde(untagged)]
enum NumericOrNull<'a, T> {
String(String),
Str(&'a str),
FromStr(T),
Null,
}
match NumericOrNull::<T>::deserialize(deserializer)? {
NumericOrNull::String(s) => match s.as_str() {
"" => Ok(None),
_ => T::from_str(&s).map(Some).map_err(serde::de::Error::custom),
},
NumericOrNull::Str(s) => match s {
"" => Ok(None),
_ => T::from_str(s).map(Some).map_err(serde::de::Error::custom),
},
NumericOrNull::FromStr(i) => Ok(Some(i)),
NumericOrNull::Null => Ok(None),
}
}
#[derive(Serialize, Deserialize, Debug, Default)]
#[derive(Serialize, Deserialize, Debug, Default, clap::Parser)]
#[clap(rename_all = "snake_case")]
pub struct TenantConfig {
#[serde(default, deserialize_with = "deserialize_option_number_from_string")]
#[clap(long)]
pub checkpoint_distance: Option<u64>,
#[clap(long)]
pub checkpoint_timeout: Option<String>,
#[serde(default, deserialize_with = "deserialize_option_number_from_string")]
#[clap(long)]
pub compaction_target_size: Option<u64>,
#[clap(long)]
pub compaction_period: Option<String>,
#[serde(default, deserialize_with = "deserialize_option_number_from_string")]
#[clap(long)]
pub compaction_threshold: Option<usize>,
#[serde(default, deserialize_with = "deserialize_option_number_from_string")]
#[clap(long)]
pub gc_horizon: Option<u64>,
#[clap(long)]
pub gc_period: Option<String>,
#[serde(default, deserialize_with = "deserialize_option_number_from_string")]
#[clap(long)]
pub image_creation_threshold: Option<usize>,
#[clap(long)]
pub pitr_interval: Option<String>,
#[clap(long)]
pub walreceiver_connect_timeout: Option<String>,
#[clap(long)]
pub lagging_wal_timeout: Option<String>,
#[serde(default, deserialize_with = "deserialize_option_number_from_string")]
#[clap(long)]
pub max_lsn_wal_lag: Option<NonZeroU64>,
#[clap(long)]
pub trace_read_requests: Option<bool>,
// We defer the parsing of the eviction_policy field to the request handler.
// Otherwise we'd have to move the types for eviction policy into this package.
// We might do that once the eviction feature has stabilizied.
// For now, this field is not even documented in the openapi_spec.yml.
#[clap(long)]
pub eviction_policy: Option<serde_json::Value>,
#[serde(default)]
#[serde(deserialize_with = "deserialize_option_number_from_string")]
#[clap(long)]
pub min_resident_size_override: Option<u64>,
#[clap(long)]
pub evictions_low_residence_duration_metric_threshold: Option<String>,
}
impl TenantConfig {
pub fn deserialize_from_settings(
settings: HashMap<&str, &str>,
) -> Result<Self, serde_json::Error> {
#[derive(Deserialize)]
#[serde(deny_unknown_fields)]
struct TenantConfigWrapper {
#[serde(flatten)]
config: TenantConfig,
}
let config = TenantConfigWrapper::deserialize(
MapDeserializer::<_, serde_json::Error>::new(settings.into_iter()),
)?;
Ok(config.config)
pub fn deserialize_from_settings(settings: HashMap<&str, &str>) -> Result<Self, anyhow::Error> {
let config = TenantConfig::try_parse_from(
std::iter::once("argv0".to_string()).chain(
settings
.iter()
.flat_map(|(k, v)| [format!("--{k}"), v.to_string()]),
),
)
.map_err(|e| {
anyhow::anyhow!(
"failed to parse: {}",
e.to_string().split('\n').next().unwrap_or_default()
) // only get the first line as other lines in clap errors are not useful
})?;
Ok(config)
}
}
@@ -878,26 +857,18 @@ mod tests {
let config = HashMap::from_iter(std::iter::once(("unknown_field", "unknown_value")));
let err = TenantConfig::deserialize_from_settings(config).unwrap_err();
assert!(
err.to_string().contains("unknown field `unknown_field`"),
err.to_string()
.contains("unexpected argument '--unknown_field' found"),
"expect unknown field `unknown_field` error, got: {}",
err
);
let config = HashMap::from_iter(std::iter::once(("checkpoint_distance", "not_a_number")));
let err = TenantConfig::deserialize_from_settings(config).unwrap_err();
assert_eq!(err.to_string(), "invalid digit found in string");
}
#[test]
fn test_deserialize_maybe_number() {
let res =
serde_json::from_str::<TenantConfig>(r#"{ "checkpoint_distance": 233 }"#).unwrap();
assert_eq!(res.checkpoint_distance, Some(233));
let res =
serde_json::from_str::<TenantConfig>(r#"{ "checkpoint_distance": "233" }"#).unwrap();
assert_eq!(res.checkpoint_distance, Some(233));
let config = HashMap::from_iter(std::iter::once(("checkpoint_distance", "233")));
let res = TenantConfig::deserialize_from_settings(config).unwrap();
assert_eq!(res.checkpoint_distance, Some(233));
assert!(
err.to_string().contains("invalid digit found in string") && err.to_string().contains("checkpoint_distance"),
"expect error to contain both 'invalid digit found in string' and the field 'checkpoint_distance', got: {}",
err
);
}
}