fix(provider-endpoints): allow disabling scheduled probes#1335
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthrough新增端点探测调度器启用开关和超时重试间隔配置,加入环境变量校验与默认值告警;禁用时跳过后台调度,但手动探测仍可执行并记录结果。测试覆盖相关配置和运行行为。 Changes端点探测调度器配置
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces configuration options to enable or disable the background endpoint probe scheduler via the ENDPOINT_PROBE_SCHEDULER_ENABLED environment variable, and to customize the timeout retry interval using ENDPOINT_PROBE_TIMEOUT_RETRY_INTERVAL_MS. It implements robust parsing helpers with fallback values and warning logs, integrates these settings into the scheduler's lifecycle, and updates the scheduler status payload. Additionally, comprehensive unit tests have been added to verify the environment configuration parsing, the behavior of a disabled scheduler, and that manual probing remains functional when the scheduler is disabled. There are no review comments, and the changes are well-implemented.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR. The implementation adds two well-validated environment variables (ENDPOINT_PROBE_SCHEDULER_ENABLED, ENDPOINT_PROBE_TIMEOUT_RETRY_INTERVAL_MS) with robust parsing, clear warning logging on invalid input, backward-compatible defaults, and thorough test coverage for the new code paths. The env-var parsing strategy (strict allowlist with fallback + warn) is actually more defensive than the existing centralized booleanTransform, and the .env.example documentation accurately reflects the new behavior.
PR Size: M
- Lines changed: 293 (285 additions, 8 deletions)
- Files changed: 4
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate
- Code clarity - Good
Automated review by Claude AI
Summary
Adds a global switch (
ENDPOINT_PROBE_SCHEDULER_ENABLED) to disable the background endpoint probe scheduler, and makes the previously-hardcoded 10s timeout-retry interval configurable (ENDPOINT_PROBE_TIMEOUT_RETRY_INTERVAL_MS). Both default to current behavior, so this change is fully backward-compatible.Problem
The endpoint probe scheduler currently starts unconditionally (from
src/instrumentation.ts) with no global off switch. In deployments with many endpoints (e.g. 100+ single-vendor endpoints), the scheduler keeps probing and writing to the probe-history table even with zero business traffic, competing for DB connections and contributing to theSomething went wrong!/CONNECT_TIMEOUT postgres:5432errors reported under load.Separately, the 10s timeout-retry interval was a hardcoded constant, so endpoints in the timeout-retry category could not be tuned via environment variables.
Related Issues:
devper the repo's PR-targeting rule. fix(provider-endpoints): allow disabling scheduled probes #1331 was closed without merge; this PR carries the same change against the correct base.ENDPOINT_PROBE_INTERVAL_MS. This PR makes the timeout interval tunable; the single-vendor interval remains fixed.Solution
startEndpointProbeScheduler(): whenENDPOINT_PROBE_SCHEDULER_ENABLED=false, it logs"Disabled by configuration"and returns early — no timer, no leader lock, no endpoint query. Existing call sites insrc/instrumentation.tsare unchanged.parseBooleanWithDefault,parsePositiveIntWithDefault) that warn and fall back to the default on invalid input, rather than silently coercing.getEndpointProbeSchedulerStatus()now exposes anenabledfield so the active mode is observable.Changes
Core Changes
src/lib/provider-endpoints/probe-scheduler.ts: newSCHEDULER_ENABLEDswitch with early-return guard;TIMEOUT_OVERRIDE_INTERVAL_MSnow read fromENDPOINT_PROBE_TIMEOUT_RETRY_INTERVAL_MS; validated env parsers with warn-on-invalid; status API gainsenabled.Supporting Changes
.env.example: documented both new variables and refreshed the surrounding comments.tests/unit/lib/provider-endpoints/probe-scheduler.test.ts(+197): env-parsing defaults / truthy / falsy / invalid-warning cases, disabled-scheduler no-op verification, custom timeout-interval probing behavior.tests/unit/lib/provider-endpoints/probe.test.ts: regression ensuring manual probe still records to history when the scheduler is disabled.Breaking Changes
None. Both new variables default to the previous behavior (
trueand10000ms). The scheduler status endpoint gains an additiveenabledfield.Testing
Automated Tests
Manual Testing
ENDPOINT_PROBE_SCHEDULER_ENABLED=falseand start the app.[EndpointProbeScheduler] Disabled by configuration.source=scheduledprobe records appear in the history table while idle.Checklist
.env.example)Description enhanced by Claude AI