Skip to content

fix: validate settings that crash at runtime instead of load time - #51

Open
rcbevans wants to merge 2 commits into
mainfrom
fix/settings-validation
Open

fix: validate settings that crash at runtime instead of load time#51
rcbevans wants to merge 2 commits into
mainfrom
fix/settings-validation

Conversation

@rcbevans

Copy link
Copy Markdown
Contributor

Summary

Nine WorkerSettings fields accepted values that passed load-time validation but crashed at runtime. Each now fails fast at settings load with a clear error message.

Fields fixed

Field Problem Fix
log_level Accepted any string Validator against {DEBUG, INFO, WARNING, ERROR, CRITICAL} (case-insensitive, normalized to uppercase)
sso_backend Accepted any string Validator against {none, oidc, saml} (case-insensitive, normalized to lowercase)
poll_interval No lower bound — 0 or negative crashes asyncio.sleep ge=0.1
notify_health_check_interval No lower bound ge=0.1
notify_reconnect_backoff_initial No lower bound ge=0.01
prune_schedule_utc Accepted any string despite "HH:MM" description Regex + range validator (00–23 hours, 00–59 minutes)
archive_expiry_schedule_utc Same Reuses _hh_mm_validator (parameterized via ctx.field_name)
prune_cron_expr Accepted any string when set croniter.is_valid() in a validator hook (skips None/empty default)
archive_expiry_cron_expr Same Reuses _cron_expr_validator

Implementation

Follows existing patterns in settings.py:

  • Validator hooks (_log_level_validator, _sso_backend_validator, _hh_mm_validator, _cron_expr_validator) match the _log_format_validator / _VALID_LOG_FORMATS pattern — raising ValueError which dotenvmodel wraps as ConstraintViolationError.
  • Field constraints (ge=0.1, ge=0.01) match existing ge= usage on other float fields.
  • croniter is already a core dependency, imported at module level.
  • The _hh_mm_validator and _cron_expr_validator are reusable — ctx.field_name produces the correct field name in each error message.

Verification

uv run ruff check .                          # All checks passed
uv run ruff format --check .                 # 475 files already formatted
uv run pyright src/taskq/settings.py         # 0 errors, 0 warnings
uv run pytest tests/test_settings.py -v      # 127 passed in 0.79s

33 new tests cover valid values, invalid values, boundary conditions, and case-insensitivity for each field.

@rcbevans
rcbevans requested review from XBeg9, clinzy and kjw-azx July 29, 2026 05:44
@rcbevans rcbevans self-assigned this Jul 29, 2026
rcbevans added 2 commits July 29, 2026 21:54
Add validators for nine WorkerSettings fields that previously accepted
invalid values silently:

- log_level: must be DEBUG/INFO/WARNING/ERROR/CRITICAL (case-insensitive)
- sso_backend: must be none/oidc/saml (case-insensitive)
- poll_interval: ge=0.1 (negative values crash asyncio.sleep)
- notify_health_check_interval: ge=0.1
- notify_reconnect_backoff_initial: ge=0.01
- prune_schedule_utc: HH:MM format with range checks
- archive_expiry_schedule_utc: same HH:MM validation
- prune_cron_expr: validated via croniter.is_valid()
- archive_expiry_cron_expr: same cron validation

Each follows the existing validator/constraint patterns in settings.py.
All existing valid values continue to load unchanged.
The ge=0.1 floor broke tests that use sub-100ms intervals for speed
(health_check_interval=0.001, poll_interval=0.05). gt=0 still prevents
the negative values that crash asyncio.sleep while allowing any positive
value for test acceleration.
@rcbevans
rcbevans force-pushed the fix/settings-validation branch from bbb9560 to f4a1bf2 Compare July 30, 2026 04:54
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.

1 participant