Skip to content

fix: Fix BeforeValidator treating 0 as falsy in configuration fields#819

Merged
vdusek merged 2 commits intomasterfrom
fix/before-validator-falsy-zero
Mar 2, 2026
Merged

fix: Fix BeforeValidator treating 0 as falsy in configuration fields#819
vdusek merged 2 commits intomasterfrom
fix/before-validator-falsy-zero

Conversation

@vdusek
Copy link
Contributor

@vdusek vdusek commented Mar 2, 2026

Summary

  • The BeforeValidator(lambda val: val or None) on max_paid_dataset_items and max_total_charge_usd configuration fields treated 0 as falsy, converting it to None. This meant a legitimate value of 0 (e.g. a zero charge limit) would be silently discarded.
  • Replaced with lambda val: val if val != '' else None which only converts empty strings (from unset env vars) to None, preserving 0 as a valid value.

Test plan

  • Added unit test verifying max_paid_dataset_items=0 is preserved (not converted to None)
  • Added unit test verifying max_total_charge_usd=0 is preserved (not converted to None)
  • Added unit test verifying empty env var for max_paid_dataset_items becomes None
  • Added unit test verifying empty env var for max_total_charge_usd becomes None
  • CI passes

🤖 Generated with Claude Code

`lambda val: val or None` coerces `0` and `Decimal(0)` to `None`,
making it impossible to set `max_paid_dataset_items=0` or
`max_total_charge_usd=0`. Use explicit empty-string check instead,
consistent with the existing `timeout_at` validator.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vdusek vdusek added adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team. labels Mar 2, 2026
@vdusek vdusek self-assigned this Mar 2, 2026
@vdusek vdusek requested a review from Pijukatel March 2, 2026 08:35
@github-actions github-actions bot added this to the 135th sprint - Tooling team milestone Mar 2, 2026
@codecov
Copy link

codecov bot commented Mar 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.85%. Comparing base (8e0e88d) to head (6e34e2a).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #819      +/-   ##
==========================================
- Coverage   85.88%   85.85%   -0.04%     
==========================================
  Files          46       46              
  Lines        2778     2778              
==========================================
- Hits         2386     2385       -1     
- Misses        392      393       +1     
Flag Coverage Δ
e2e 37.07% <ø> (ø)
integration 58.60% <ø> (-0.04%) ⬇️
unit 73.39% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions bot added the tested Temporary label used only programatically for some analytics. label Mar 2, 2026
@vdusek vdusek merged commit 72efe88 into master Mar 2, 2026
31 checks passed
@vdusek vdusek deleted the fix/before-validator-falsy-zero branch March 2, 2026 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants