Uh oh!
There was an error while loading. Please reload this page.
chore(config): remove config field from Reloader - #1293
Conversation
This field is only really used at startup and then simply becomes a thing to be kept in memory with no additional value, the internal watch channels `Reloader` uses already has owned copies of everything in there.
The tests themselves are pretty basic, they simply: * Create a `FactConfig` object. * Turn it into a `Reloader`. * Feed a new `FactConfig` object to the `Reloader`. * Check the correspoding `watch::Receiver` to see the expected update. These tests show some inconsistencies on how updating from or to explicit versions of default values behave, some fields send an update and some don't. We might want to address this at some point. A new approach is also used for the added tests, instead of using an array for defining the cases and iterating over them, the main logic is abstracted in a macro and invoked for each set of input/expected values. This gives a few benefits: * Each test case gets a dedicated test name, making it easier to find where an error occurs. * If one test case fails, the rest still run (the array iteration approach stops at the first failure). * All test cases are run in parallel, potentially speeding up the tests (though the current state of tests is fast enough for now). The downside is the number of tests bloats quite a bit, but this should not be too much of an issue. This new approach is properly described here: https://unterwaditzer.net/2023/rust-test-parametrization.html
📝 WalkthroughWalkthroughThe reloader now stores startup hotreload state and propagates field-specific configuration updates through watch channels. Pipeline setup passes BPF and runtime state through ChangesReloader and pipeline refactor
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
codecov-commenter
commented
Jul 27, 2026
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@## main #1293 +/- ##
==========================================
+ Coverage 32.73% 35.97% +3.24%
==========================================
Files 22 22 Lines 3174 3180 +6 Branches 3174 3180 +6 ==========================================
+ Hits 1039 1144 +105 + Misses 2131 2033 -98 + Partials 4 3 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Description
This field is only really used at startup and then simply becomes a thing to be kept in memory with no additional value, the internal watch channels
Reloaderuses already has owned copies of everything in there.Checklist
Automated testing
If any of these don't apply, please comment below.
Testing Performed
Added unit tests to
config::reloader::Reloader.The tests themselves are pretty basic, they simply:
FactConfigobject.Reloader.FactConfigobject to theReloader.watch::Receiverto see the expected update.These tests show some inconsistencies on how updating from or to
explicit versions of default values behave, some fields send an update
and some don't. We might want to address this at some point.
A new approach is also used for the added tests, instead of using an
array for defining the cases and iterating over them, the main logic is
abstracted in a macro and invoked for each set of input/expected values.
This gives a few benefits:
where an error occurs.
approach stops at the first failure).
(though the current state of tests is fast enough for now).
The downside is the number of tests bloats quite a bit, but this should
not be too much of an issue.
This new approach is properly described here: https://unterwaditzer.net/2023/rust-test-parametrization.html
Summary by CodeRabbit
Bug Fixes
Refactor
Tests