From 4bcc010e087c7e01b086026ae5793c382fa34aa1 Mon Sep 17 00:00:00 2001 From: Pierre Chevalier Date: Thu, 18 Jul 2024 06:50:19 -0700 Subject: [PATCH] Fix bug which prevented the static cached config from being used Summary: JustKnobs provides two ways to `init` with a `cached_config`: * [`init_just_knobs`](https://www.internalfb.com/intern/rustdoc/fbcode/common/rust/shed/justknobs_stub:justknobs/justknobs/cached_config/fn.init_just_knobs.html) * [`init_just_knobs_worker`](https://www.internalfb.com/intern/rustdoc/fbcode/common/rust/shed/justknobs_stub:justknobs/justknobs/cached_config/fn.init_just_knobs_worker.html) The former is simple and stores the provided config as a static `HashMap` of knobs. The latter is fancier: given a tokio runtime, it starts a thread that watches a file for changes. The former is good enough for test situations where we just want to parse some json and run tests with it. But... it is unusable (prior to this change) as for a static config, we never declare it to be "`in_use`", which means that when we run `eval`, we fall back to the actual prod jk despite initializing the simple static justknobs. Not anymore. Reviewed By: YousefSalama Differential Revision: D59867073 fbshipit-source-id: aaeb0d94b158c840ba34b34c15ac050fe4885205 --- shed/justknobs_stub/src/cached_config.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/shed/justknobs_stub/src/cached_config.rs b/shed/justknobs_stub/src/cached_config.rs index a798721ea..edafa9aa8 100644 --- a/shed/justknobs_stub/src/cached_config.rs +++ b/shed/justknobs_stub/src/cached_config.rs @@ -54,7 +54,7 @@ enum KnobVal { } pub fn in_use() -> bool { - JUST_KNOBS_WORKER_STATE.get().is_some() + JUST_KNOBS_WORKER_STATE.get().is_some() || JUST_KNOBS.get().is_some() } pub fn just_knobs() -> &'static ArcSwap { @@ -245,11 +245,7 @@ mod test { init_just_knobs(&logger, &config_handle)?; assert!(CachedConfigJustKnobs::eval("my/config:knob1", None, None).unwrap()); - // This is a problem: the public interface for interacting with this jk errs, despite being - // initialized with a static config handle that contains this justknob. - assert!(justknobs::eval("my/config:knob1", None, None).is_err()); - // We should expect this to work: - // assert!(justknobs::eval("my/config:knob1", None, None).unwrap()); + assert!(justknobs::eval("my/config:knob1", None, None).unwrap()); Ok(()) } }