Skip to content

fix: avoid config watchers in static config tests#4101

Open
immanuwell wants to merge 1 commit into
ory:masterfrom
immanuwell:fix-static-config-file-watchers
Open

fix: avoid config watchers in static config tests#4101
immanuwell wants to merge 1 commit into
ory:masterfrom
immanuwell:fix-static-config-file-watchers

Conversation

@immanuwell

@immanuwell immanuwell commented May 8, 2026

Copy link
Copy Markdown

Related issue(s)

No direct issue found. Closest hits were #47 and #2850, but different root causes.

What changed

TestProviderValidates only reads internal/.hydra.yaml as a static fixture, but configx still spins up an fsnotify watcher for it. On machines already near fs.inotify.max_user_instances, that goes boom with too many open files.

Repro:

sysctl fs.inotify.max_user_instances
go test ./driver/config -run TestProviderValidates -count=1 -v

This adds configx.DisableFileWatching() and uses it for that static config test. Prod defaults stay the same; file watching is still on unless callers opt out. Tiny deflake, no big machinery.

Checks:

go test ./driver/config -run TestProviderValidates -count=1 -v
go test ./driver/config -count=1
(cd oryx && go test ./configx -count=1)
git diff --check

Checklist

  • I have read the contributing guidelines.
  • I am following the contributing code guidelines.
  • I have read the security policy.
  • I confirm that this pull request does not address a security vulnerability.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added or changed the documentation.

Summary by CodeRabbit

  • New Features
    • Added configuration option to disable file watching, allowing configuration files to be loaded once without automatic change monitoring.

@immanuwell immanuwell requested review from a team and aeneasr as code owners May 8, 2026 15:58
@CLAassistant

CLAassistant commented May 8, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@coderabbitai

coderabbitai Bot commented May 8, 2026

Copy link
Copy Markdown

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a4caf8f0-d588-40ee-baf8-2ab267859a7c

📥 Commits

Reviewing files that changed from the base of the PR and between a54baeb and 3bb462d.

📒 Files selected for processing (3)
  • driver/config/provider_test.go
  • oryx/configx/options.go
  • oryx/configx/provider.go

📝 Walkthrough

Walkthrough

This PR adds a DisableFileWatching() configuration option to the Provider, allowing callers to load configuration files once without spawning a file-watching goroutine. The option is integrated into Provider initialization to skip watcher startup and per-file channel registration. A test is updated to exercise the new behavior.

Changes

File Watching Disable Feature

Layer / File(s) Summary
Configuration Option Interface
oryx/configx/options.go
Introduces DisableFileWatching() OptionModifier function to disable file watching.
Provider Implementation
oryx/configx/provider.go
Adds disableFileWatching boolean field to Provider struct and integrates it to skip watcher goroutine startup and per-file WatchChannel registration.
Test Usage
driver/config/provider_test.go
Updates TestProviderValidates to apply DisableFileWatching() option during provider construction.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding a mechanism to disable file watchers in static config tests to avoid inotify exhaustion.
Description check ✅ Passed The description provides context on the problem, reproduction steps, the solution, and verification checks. Most checklist items are completed, though documentation was not updated.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants