Skip to content

refactor: consolidate empty-string env-var validators in Configuration#965

Merged
vdusek merged 3 commits into
masterfrom
fix/empty-string-json-env-vars
Jun 16, 2026
Merged

refactor: consolidate empty-string env-var validators in Configuration#965
vdusek merged 3 commits into
masterfrom
fix/empty-string-json-env-vars

Conversation

@vdusek

@vdusek vdusek commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Description

A few Configuration fields were already guarded against empty-string env vars with ad-hoc BeforeValidator lambdas. The Apify platform sometimes sets an env var to an empty string instead of leaving it unset (see #303, or this Discord thread), and for these fields '' cannot be parsed into the target type, so without the guard validation would raise and crash Actor.init().

This PR is a pure refactor: it consolidates those repeated lambdas into a single shared _default_if_empty(default=...) validator. There is no behavior change, and the empty-string handling is not extended to any new fields.

Changes

  • Added a shared _default_if_empty(default=...) validator.
  • Replaced the existing ad-hoc lambdas with it on the four fields that already handled '':
    • max_paid_dataset_items (→ None)
    • max_total_charge_usd (→ None)
    • timeout_at (→ None)
    • user_is_paying (→ False)
  • Added a parametrized regression test covering those four fields.

@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 Jun 11, 2026
@vdusek vdusek self-assigned this Jun 11, 2026
@codecov

codecov Bot commented Jun 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.33%. Comparing base (0daca28) to head (6c90a1d).
⚠️ Report is 17 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #965      +/-   ##
==========================================
+ Coverage   89.90%   90.33%   +0.42%     
==========================================
  Files          49       49              
  Lines        3091     3093       +2     
==========================================
+ Hits         2779     2794      +15     
+ Misses        312      299      -13     
Flag Coverage Δ
e2e 36.27% <80.00%> (+0.36%) ⬆️
integration 57.48% <100.00%> (+0.60%) ⬆️
unit 78.95% <100.00%> (+0.20%) ⬆️

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

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

@vdusek vdusek requested a review from Pijukatel June 11, 2026 13:15
@vdusek vdusek marked this pull request as ready for review June 11, 2026 13:15
Comment thread tests/unit/actor/test_configuration.py Outdated
@github-actions github-actions Bot added this to the 142nd sprint - Tooling team milestone Jun 12, 2026
@github-actions github-actions Bot added the tested Temporary label used only programatically for some analytics. label Jun 12, 2026
@vdusek vdusek marked this pull request as draft June 14, 2026 16:43
@vdusek vdusek changed the title fix: treat empty-string JSON env vars as absent in Configuration fix: treat empty-string env vars as absent in Configuration Jun 14, 2026
@vdusek vdusek changed the title fix: treat empty-string env vars as absent in Configuration chore: treat empty-string env vars as absent in Configuration Jun 15, 2026
@vdusek vdusek marked this pull request as ready for review June 16, 2026 07:11
@vdusek vdusek requested a review from Pijukatel June 16, 2026 07:12
@vdusek vdusek force-pushed the fix/empty-string-json-env-vars branch from 5cc2a2a to 6c90a1d Compare June 16, 2026 09:12
@vdusek vdusek changed the title chore: treat empty-string env vars as absent in Configuration refactor: consolidate empty-string env-var validators in Configuration Jun 16, 2026
@vdusek vdusek merged commit b609955 into master Jun 16, 2026
51 of 53 checks passed
@vdusek vdusek deleted the fix/empty-string-json-env-vars branch June 16, 2026 09:58
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.

3 participants