fix: treat empty-string env vars as unset in Configuration - #1080
Closed
vdusek wants to merge 3 commits into
Closed
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1080 +/- ##
==========================================
+ Coverage 91.83% 91.89% +0.05%
==========================================
Files 51 51
Lines 3235 3232 -3
==========================================
- Hits 2971 2970 -1
+ Misses 264 262 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Author
|
closing as duplicate of #965 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
The Apify platform sometimes exports an env var as an empty string instead of leaving it unset.
Configurationhandled that per field, withBeforeValidator(_default_if_empty(...))on four fields — so every other field whose type cannot parse''raised a raw pydanticValidationErrorwhileConfigurationwas being built insideActor.init(), before logging was configured.Verified crashing on
'':ACTOR_STORAGES_JSON,ACTOR_STARTED_AT,APIFY_DEDICATED_CPUS,ACTOR_TEST_PAY_PER_EVENT,APIFY_IS_AT_HOME,ACTOR_STANDBY_URL,APIFY_METAMORPH_AFTER_SLEEP_MILLIS,APIFY_PROXY_PORT,ACTOR_WEB_SERVER_PORT,APIFY_CHARGED_ACTOR_EVENT_COUNTS, plus inherited Crawlee fields such asCRAWLEE_PURGE_ON_STARTandCRAWLEE_MEMORY_MBYTES.The fix
Drop the per-field annotations and the
_default_if_emptyhelper, and setenv_ignore_empty=Trueinmodel_configinstead. The empty value is then skipped in the env settings source, so the declared field default applies and no field can be forgotten.Skipping in the env source matters, rather than normalizing after the sources are merged: 28 of 55 fields accept several env names via
AliasChoices, and only the source-level skip lets the next name be tried. WithACTOR_WEB_SERVER_PORT=''besideAPIFY_CONTAINER_PORT=9999, the port resolves to9999; discarding the merged value would silently yield the4321default. It also keeps the rule scoped to the environment — an''passed to the constructor is still taken as given.pydantic-settingsmoves from a transitive dependency (viacrawlee) to a declared one, since_configuration.pynow importsSettingsConfigDictdirectly.Observable changes
''now means "not provided" for every field, not just the four.APIFY_TOKEN=''yieldsNoneinstead of'',ACTOR_INPUT_KEY=''yields'INPUT'instead of an unusable'', andACTOR_BUILD_TAGS=''yieldsNoneinstead of[]. Nothing in the SDK or Crawlee distinguishes''fromNoneon these fields.actor_task_id's description is updated accordingly.Tests
The empty-string regression test grows from 4 to 16 cases, including a Crawlee-inherited field. Added alongside it: a guard that a populated typed env var is still parsed (so the rule cannot become greedy), a guard that an empty value falls through to a populated legacy alias, and a guard that a constructor-supplied
''is kept. 13 of these fail onmaster.✍️ Drafted by Claude Code