Skip to content

Commit

Permalink
Fix bug which prevented the static cached config from being used
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Pierre Chevalier authored and facebook-github-bot committed Jul 18, 2024
1 parent 6da569e commit 4bcc010
Showing 1 changed file with 2 additions and 6 deletions.
8 changes: 2 additions & 6 deletions shed/justknobs_stub/src/cached_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<JustKnobsInMemory> {
Expand Down Expand Up @@ -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(())
}
}

0 comments on commit 4bcc010

Please sign in to comment.