[JoukoVirtanen]

Description

Rate limiting for Fact has been implemented, but it is not enabled by default. This PR sets the default rate limit to 400 events per second.

Checklist

• Patch has a change log entry OR does not need one.
• Investigated and inspected CI test results
• Updated documentation accordingly

Automated testing

• Added unit tests
• Added integration tests
• Added regression tests

If any of these don't apply, please comment below.

Testing Performed

CI failures are not related to the changes in this PR.

TODO(replace-me)
Use this space to explain how you tested your PR, or, if you didn't test it, why you did not do so. (Valid reasons include "CI is sufficient" or "No testable changes")
In addition to reviewing your code, reviewers must also review your testing instructions, and make sure they are sufficient.

For more details, ref the Confluence page about this section.

Summary by CodeRabbit

• New Features

  • Added a default rate limit of 400 events per second when no limit is configured.
  • Explicit rate-limit settings remain unchanged, including 0 for unlimited operation.

• Documentation

  • Updated CLI documentation to state the default rate limit of 400 events per second.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: stackrox/fact/.coderabbit.yml
• Review profile: CHILL
• Plan: Enterprise
• Run ID: ef72a9e7-8f6e-4f86-86ae-2e51c1d41f99

📥 Commits

Reviewing files that changed from the base of the PR and between 77fbaeb and d6009e4.

📒 Files selected for processing: 3

• fact/src/config/mod.rs
• fact/src/config/reloader/tests.rs
• fact/src/config/tests.rs

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

📝 Walkthrough

An unset rate limit now defaults to 400 events per second. Explicit values remain unchanged, including zero for unlimited. The CLI documentation and configuration tests reflect the updated default.

Changes: Rate Limit Default

Layer / File(s)	Summary
Set and validate the rate limit default fact/src/config/mod.rs, fact/src/config/reloader/tests.rs, fact/src/config/tests.rs	FactConfig::rate_limit() now uses DEFAULT_RATE_LIMIT (400) when unset. The CLI documentation and tests use the updated default. Reloader tests retain the expected behavior for transitions between implicit and explicit defaults.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Feature

Suggested reviewers: molter73

Merge Risk: ⚪ Minimal · up to d6009

The rate-limit default change is consistent across configuration, runtime behavior, documentation, and tests.

Pre-merge checks: 3 · 2

Failed checks: 2 warnings

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check		The description explains the rate-limit change and includes the required sections. However, the checklist remains unchecked, and the Testing Performed section still contains TODO(replace-me) without t…	Complete the checklist. Replace TODO(replace-me) with specific testing performed, or state why testing was not performed. Confirm whether a changelog entry, documentation update, and automated tests are required, and mark each applicable it…

Passed checks: 3 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: setting Fact's default rate limit.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description explains the rate-limit change and includes the required sections. However, the checklist remains unchecked, and the Testing Performed section still contains TODO(replace-me) without test steps or a clear reason for not providing them.

Resolution

Complete the checklist. Replace TODO(replace-me) with specific testing performed, or state why testing was not performed. Confirm whether a changelog entry, documentation update, and automated tests are required, and mark each applicable item accordingly.

✨ Finishing Touches: 💡 2

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

_{Comment @coderabbitai help to get the list of available commands.}

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 33.45%. Comparing base (77fbaeb) to head (d6009e4).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2197   +/-   ##
=======================================
  Coverage   33.45%   33.45%           
=======================================
  Files          22       22           
  Lines        3623     3623           
  Branches     3623     3623           
=======================================
  Hits         1212     1212           
  Misses       2402     2402           
  Partials        9        9           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Molter73]

I don't think rate limiting is needed by default. Fact is designed to monitor paths that are rarely changed and should not be sending hundreds (maybe not event tens) of events a second, the rate limiter is there for people that want to use fact in a way it is not intended to be used as, not for us to drop events on people's behalf.