From d7be894c00a74067bd30bcdd94e1900257136ef1 Mon Sep 17 00:00:00 2001 From: Steven Malis Date: Tue, 17 Dec 2024 13:09:58 -0500 Subject: [PATCH 1/2] flowey: Avoid calling env::set_var --- flowey/flowey_cli/src/cli/mod.rs | 17 ++++++++++------- support/ci_logger/src/lib.rs | 15 +++++++++++++++ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/flowey/flowey_cli/src/cli/mod.rs b/flowey/flowey_cli/src/cli/mod.rs index 2f4f2bf77f..a07e99ef61 100644 --- a/flowey/flowey_cli/src/cli/mod.rs +++ b/flowey/flowey_cli/src/cli/mod.rs @@ -51,13 +51,18 @@ pub fn cli_main( // This mechanism is in-place because some YAML pipeline defn langs (*cough* // ADO *cough*) don't let you set pipeline-level env vars which are // automatically inherited by all shell contexts. + let mut log_override = None; if let Commands::VarDb(var_db::VarDb { job_idx, .. }) | Commands::ExecSnippet(exec_snippet::ExecSnippet { job_idx, .. }) = &cli.command { - let _ = try_inject_flowey_log(*job_idx); + log_override = try_get_flowey_log(*job_idx).unwrap_or_default(); } - ci_logger::init("FLOWEY_LOG").unwrap(); + if let Some(log_level) = log_override { + ci_logger::init_value(&log_level).unwrap(); + } else { + ci_logger::init("FLOWEY_LOG").unwrap(); + } match cli.command { Commands::Debug(cmd) => cmd.run(), @@ -85,7 +90,7 @@ impl From for FlowBackend { } } -fn try_inject_flowey_log(job_idx: usize) -> anyhow::Result<()> { +fn try_get_flowey_log(job_idx: usize) -> anyhow::Result> { // skip if the env var is already set if std::env::var("FLOWEY_LOG").is_err() { let log_level = var_db::open_var_db(job_idx)? @@ -96,11 +101,9 @@ fn try_inject_flowey_log(job_idx: usize) -> anyhow::Result<()> { }); if let Some(log_level) = log_level { - // Yes, this is a hack... but I kinda don't want to go update the - // ci_logger crate right now - std::env::set_var("FLOWEY_LOG", log_level) + return Ok(Some(log_level)); } } - Ok(()) + Ok(None) } diff --git a/support/ci_logger/src/lib.rs b/support/ci_logger/src/lib.rs index f1f30ae24f..620c9bd91e 100644 --- a/support/ci_logger/src/lib.rs +++ b/support/ci_logger/src/lib.rs @@ -28,6 +28,15 @@ impl AdoLogger { filter, } } + + fn new_value(log_level: &str) -> AdoLogger { + let filter = env_logger::filter::Builder::new().parse(log_level).build(); + + AdoLogger { + in_ci: std::env::var("TF_BUILD").is_ok(), + filter, + } + } } impl log::Log for AdoLogger { @@ -83,3 +92,9 @@ pub fn init(log_env_var: &str) -> Result<(), log::SetLoggerError> { log::set_boxed_logger(Box::new(AdoLogger::new(log_env_var))) .map(|()| log::set_max_level(log::LevelFilter::Trace)) } + +/// Initialize the ADO logger with a specific value, instead of an env var. +pub fn init_value(log_level: &str) -> Result<(), log::SetLoggerError> { + log::set_boxed_logger(Box::new(AdoLogger::new_value(log_level))) + .map(|()| log::set_max_level(log::LevelFilter::Trace)) +} From 7eaa65224ae5952b1b7556d99257824bbbce56f1 Mon Sep 17 00:00:00 2001 From: Steven Malis Date: Tue, 17 Dec 2024 13:52:22 -0500 Subject: [PATCH 2/2] Feedback --- flowey/flowey_cli/src/cli/mod.rs | 2 +- support/ci_logger/src/lib.rs | 25 +++++++++---------------- 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/flowey/flowey_cli/src/cli/mod.rs b/flowey/flowey_cli/src/cli/mod.rs index a07e99ef61..f240ce03a9 100644 --- a/flowey/flowey_cli/src/cli/mod.rs +++ b/flowey/flowey_cli/src/cli/mod.rs @@ -59,7 +59,7 @@ pub fn cli_main( } if let Some(log_level) = log_override { - ci_logger::init_value(&log_level).unwrap(); + ci_logger::init_with_level(&log_level).unwrap(); } else { ci_logger::init("FLOWEY_LOG").unwrap(); } diff --git a/support/ci_logger/src/lib.rs b/support/ci_logger/src/lib.rs index 620c9bd91e..b15a2984d7 100644 --- a/support/ci_logger/src/lib.rs +++ b/support/ci_logger/src/lib.rs @@ -14,10 +14,10 @@ struct AdoLogger { } impl AdoLogger { - fn new(log_env_var: &str) -> AdoLogger { + fn new(log_level: Option<&str>) -> AdoLogger { let mut builder = env_logger::filter::Builder::new(); - if let Ok(var) = std::env::var(log_env_var) { - builder.parse(&var); + if let Some(log_level) = log_level { + builder.parse(log_level); } else { builder.filter_level(log::LevelFilter::Info); } @@ -28,15 +28,6 @@ impl AdoLogger { filter, } } - - fn new_value(log_level: &str) -> AdoLogger { - let filter = env_logger::filter::Builder::new().parse(log_level).build(); - - AdoLogger { - in_ci: std::env::var("TF_BUILD").is_ok(), - filter, - } - } } impl log::Log for AdoLogger { @@ -89,12 +80,14 @@ impl log::Log for AdoLogger { /// Initialize the ADO logger pub fn init(log_env_var: &str) -> Result<(), log::SetLoggerError> { - log::set_boxed_logger(Box::new(AdoLogger::new(log_env_var))) - .map(|()| log::set_max_level(log::LevelFilter::Trace)) + log::set_boxed_logger(Box::new(AdoLogger::new( + std::env::var(log_env_var).ok().as_deref(), + ))) + .map(|()| log::set_max_level(log::LevelFilter::Trace)) } /// Initialize the ADO logger with a specific value, instead of an env var. -pub fn init_value(log_level: &str) -> Result<(), log::SetLoggerError> { - log::set_boxed_logger(Box::new(AdoLogger::new_value(log_level))) +pub fn init_with_level(log_level: &str) -> Result<(), log::SetLoggerError> { + log::set_boxed_logger(Box::new(AdoLogger::new(Some(log_level)))) .map(|()| log::set_max_level(log::LevelFilter::Trace)) }