RHINENG-25511: add ff to disable advisory_account_data update in eval… - #2344
Conversation
Reviewer's GuideIntroduces the File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="evaluator/evaluate_advisories.go" line_range="420-423" />
<code_context>
system *models.SystemPlatformV2,
advisoriesByName extendedAdvisoryMap,
) error {
+ if !enableAdvisoryAccountData {
+ utils.LogInfo("inventoryID", system.GetInventoryID(), "advisory_account_data updates disabled, skipping")
+ return nil
+ }
+
changes := calcAdvisoryChanges(system, advisoriesByName)
</code_context>
<issue_to_address>
**issue (testing):** The new package-level flag defaults to false until `configureEvaluator` runs, so direct calls to `updateAdvisoryAccountData` in the existing database test now return without changing counts; `TestUpdateAdvisoryAccountData` consequently fails its expected 0/1 account-data assertions because the evaluator configuration is not initialized by the test suite.
**Triggers:** When evaluator tests invoke `updateAdvisoryAccountData` directly without calling `configureEvaluator`.
**Suggested fix:** Initialize the flag to its documented default (`true`) or explicitly enable it in tests that exercise the default update behavior.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and when the flag is disabled, advisory_account_data counts are not updated and can remain stale in the database. Reverting restores future updates, and the missed counts are bounded and can be repaired by rerunning the evaluation.
Blocking findings: evaluator/evaluate_advisories.go:423
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2344 +/- ##
==========================================
- Coverage 58.94% 58.92% -0.02%
==========================================
Files 150 150
Lines 9601 9605 +4
==========================================
+ Hits 5659 5660 +1
- Misses 3348 3351 +3
Partials 594 594
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…uator
Secure Coding Practices Checklist GitHub Link
Secure Coding Checklist
Summary by Sourcery
Add a feature flag to disable legacy advisory account data updates during evaluation.
New Features:
Enhancements:
Tests: