diff --git a/pyproject.toml b/pyproject.toml index 012d7074..e908dae5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -43,6 +43,7 @@ dependencies = [ "lazy-object-proxy>=1.11.0", "more_itertools>=10.2.0", "pydantic>=2.11.0", + "pydantic-settings>=2.12.0", "typing-extensions>=4.4.0", "websockets>=14.0", "yarl>=1.18.0", diff --git a/src/apify/_configuration.py b/src/apify/_configuration.py index b54ad08a..545e7408 100644 --- a/src/apify/_configuration.py +++ b/src/apify/_configuration.py @@ -5,10 +5,11 @@ from decimal import Decimal from logging import getLogger from pathlib import Path -from typing import TYPE_CHECKING, Annotated, Any, Self +from typing import Annotated, Any, Self -from pydantic import AliasChoices, AliasGenerator, BeforeValidator, ConfigDict, Field, model_validator +from pydantic import AliasChoices, AliasGenerator, BeforeValidator, Field, model_validator from pydantic.alias_generators import to_camel +from pydantic_settings import SettingsConfigDict from typing_extensions import TypedDict from crawlee import service_locator @@ -24,9 +25,6 @@ ) from apify._utils import docs_group -if TYPE_CHECKING: - from collections.abc import Callable - logger = getLogger(__name__) @@ -38,20 +36,6 @@ def _transform_to_list(value: Any) -> list[str] | None: return value if isinstance(value, list) else str(value).split(',') -def _default_if_empty(*, default: Any) -> Callable[[Any], Any]: - """Build a validator that substitutes `default` for an empty-string env var. - - The Apify platform sometimes sets an env var to an empty string instead of leaving it unset. For fields whose - target type cannot parse `''` (datetimes, numbers, booleans, ...), passing the value straight through would crash - validation and, in turn, `Actor.init()`. Treat `''` as "not provided" and fall back to the field default instead. - """ - - def transform(value: Any) -> Any: - return default if value == '' else value - - return transform - - class ActorStorages(TypedDict): """Mapping of storage aliases to their IDs, grouped by storage type. @@ -115,7 +99,15 @@ class Configuration(CrawleeConfiguration): # Fields are validated from environment variables via their `validation_alias`, but serialized under a # camelCase name derived from the Python field name. This keeps `model_dump(by_alias=True)` consistent # (e.g. `is_at_home` -> `isAtHome`) instead of leaking the raw env-var names. - model_config = ConfigDict(alias_generator=AliasGenerator(serialization_alias=to_camel)) + # + # `env_ignore_empty` makes an env var exported as an empty string count as unset. The Apify platform sometimes + # does that instead of omitting the variable, and fields whose target type cannot parse `''` (datetimes, numbers, + # booleans, JSON, URLs, ...) would otherwise fail validation and crash `Actor.init()`. Skipping the value in the + # env source - rather than after the sources are merged - also lets the next name in an `AliasChoices` be tried. + model_config = SettingsConfigDict( + alias_generator=AliasGenerator(serialization_alias=to_camel), + env_ignore_empty=True, + ) actor_id: Annotated[ str | None, @@ -192,7 +184,7 @@ class Configuration(CrawleeConfiguration): 'actor_task_id', 'apify_actor_task_id', ), - description='ID of the Actor task. Empty if Actor is run outside of any task, e.g. directly using the API', + description='ID of the Actor task. None if Actor is run outside of any task, e.g. directly using the API', ), ] = None @@ -306,7 +298,6 @@ class Configuration(CrawleeConfiguration): validation_alias='actor_max_paid_dataset_items', description='For paid-per-result Actors, the user-set limit on returned results. Do not exceed this limit', ), - BeforeValidator(_default_if_empty(default=None)), ] = None max_total_charge_usd: Annotated[ @@ -315,7 +306,6 @@ class Configuration(CrawleeConfiguration): validation_alias='actor_max_total_charge_usd', description='For pay-per-event Actors, the user-set limit on total charges. Do not exceed this limit', ), - BeforeValidator(_default_if_empty(default=None)), ] = None test_pay_per_event: Annotated[ @@ -394,7 +384,6 @@ class Configuration(CrawleeConfiguration): ), description='Date when the Actor will time out', ), - BeforeValidator(_default_if_empty(default=None)), ] = None standby_url: Annotated[ @@ -428,7 +417,6 @@ class Configuration(CrawleeConfiguration): validation_alias='apify_user_is_paying', description='True if the user calling the Actor is paying user', ), - BeforeValidator(_default_if_empty(default=False)), ] = False web_server_port: Annotated[ diff --git a/tests/unit/actor/test_actor_env_helpers.py b/tests/unit/actor/test_actor_env_helpers.py index 0d5adfdb..e78adde3 100644 --- a/tests/unit/actor/test_actor_env_helpers.py +++ b/tests/unit/actor/test_actor_env_helpers.py @@ -166,8 +166,10 @@ async def test_get_env_with_randomized_env_vars(monkeypatch: pytest.MonkeyPatch, list_get_env_var = list_env_var.name.lower() expected_value_count = random.randint(0, len(available_values)) - expected_get_env[list_get_env_var] = random.sample(available_values, expected_value_count) - monkeypatch.setenv(list_env_var, ','.join(expected_get_env[list_get_env_var])) + expected_values = random.sample(available_values, expected_value_count) + monkeypatch.setenv(list_env_var, ','.join(expected_values)) + # An env var exported as an empty string counts as unset, same as a missing one. + expected_get_env[list_get_env_var] = expected_values or None # Test behavior with missing env var in case of empty list if expected_value_count == 0 and random.random() < 0.5: diff --git a/tests/unit/actor/test_configuration.py b/tests/unit/actor/test_configuration.py index 486cbe07..53d852c9 100644 --- a/tests/unit/actor/test_configuration.py +++ b/tests/unit/actor/test_configuration.py @@ -1,11 +1,13 @@ +from __future__ import annotations + import json +from datetime import UTC, datetime, timedelta from decimal import Decimal -from pathlib import Path +from typing import TYPE_CHECKING import pytest from crawlee import Request, service_locator -from crawlee._types import BasicCrawlingContext from crawlee.configuration import Configuration as CrawleeConfiguration from crawlee.crawlers import BasicCrawler from crawlee.errors import ServiceConflictError @@ -14,6 +16,11 @@ from apify import Configuration as ApifyConfiguration from apify.storage_clients._smart_apify._storage_client import SmartApifyStorageClient +if TYPE_CHECKING: + from pathlib import Path + + from crawlee._types import BasicCrawlingContext + @pytest.mark.parametrize( ('is_at_home', 'disable_browser_sandbox_in', 'disable_browser_sandbox_out'), @@ -290,20 +297,6 @@ def test_max_total_charge_usd_zero_is_preserved(monkeypatch: pytest.MonkeyPatch) assert config.max_total_charge_usd == Decimal(0) -def test_max_paid_dataset_items_empty_string_becomes_none(monkeypatch: pytest.MonkeyPatch) -> None: - """Test that an empty env var for max_paid_dataset_items is converted to None.""" - monkeypatch.setenv('ACTOR_MAX_PAID_DATASET_ITEMS', '') - config = ApifyConfiguration() - assert config.max_paid_dataset_items is None - - -def test_max_total_charge_usd_empty_string_becomes_none(monkeypatch: pytest.MonkeyPatch) -> None: - """Test that an empty env var for max_total_charge_usd is converted to None.""" - monkeypatch.setenv('ACTOR_MAX_TOTAL_CHARGE_USD', '') - config = ApifyConfiguration() - assert config.max_total_charge_usd is None - - def test_max_total_charge_usd_decimal_parsing(monkeypatch: pytest.MonkeyPatch) -> None: """Test that max_total_charge_usd is parsed as Decimal from env var.""" from decimal import Decimal @@ -397,10 +390,28 @@ def test_actor_storage_json_env_var(monkeypatch: pytest.MonkeyPatch) -> None: @pytest.mark.parametrize( ('env_var', 'attr', 'expected'), [ - ('APIFY_TIMEOUT_AT', 'timeout_at', None), - ('ACTOR_MAX_PAID_DATASET_ITEMS', 'max_paid_dataset_items', None), - ('ACTOR_MAX_TOTAL_CHARGE_USD', 'max_total_charge_usd', None), - ('APIFY_USER_IS_PAYING', 'user_is_paying', False), + pytest.param('APIFY_TIMEOUT_AT', 'timeout_at', None, id='timeout_at'), + pytest.param('ACTOR_MAX_PAID_DATASET_ITEMS', 'max_paid_dataset_items', None, id='max_paid_dataset_items'), + pytest.param('ACTOR_MAX_TOTAL_CHARGE_USD', 'max_total_charge_usd', None, id='max_total_charge_usd'), + pytest.param('APIFY_USER_IS_PAYING', 'user_is_paying', False, id='user_is_paying'), + pytest.param('ACTOR_STORAGES_JSON', 'actor_storages', None, id='actor_storages'), + pytest.param('ACTOR_STARTED_AT', 'started_at', None, id='started_at'), + pytest.param('APIFY_DEDICATED_CPUS', 'dedicated_cpus', None, id='dedicated_cpus'), + pytest.param('ACTOR_TEST_PAY_PER_EVENT', 'test_pay_per_event', False, id='test_pay_per_event'), + pytest.param('APIFY_IS_AT_HOME', 'is_at_home', False, id='is_at_home'), + pytest.param('ACTOR_STANDBY_URL', 'standby_url', 'http://localhost', id='standby_url'), + pytest.param( + 'APIFY_METAMORPH_AFTER_SLEEP_MILLIS', + 'metamorph_after_sleep', + timedelta(minutes=5), + id='metamorph_after_sleep', + ), + pytest.param('APIFY_PROXY_PORT', 'proxy_port', 8000, id='proxy_port'), + pytest.param('ACTOR_WEB_SERVER_PORT', 'web_server_port', 4321, id='web_server_port'), + pytest.param('APIFY_CHARGED_ACTOR_EVENT_COUNTS', 'charged_event_counts', None, id='charged_event_counts'), + pytest.param('ACTOR_INPUT_KEY', 'input_key', 'INPUT', id='input_key'), + pytest.param('ACTOR_BUILD_TAGS', 'actor_build_tags', None, id='actor_build_tags'), + pytest.param('CRAWLEE_PURGE_ON_START', 'purge_on_start', True, id='purge_on_start'), ], ) def test_typed_env_var_empty_string_falls_back_to_default( @@ -410,3 +421,69 @@ def test_typed_env_var_empty_string_falls_back_to_default( monkeypatch.setenv(env_var, '') config = ApifyConfiguration() assert getattr(config, attr) == expected + + +@pytest.mark.parametrize( + ('env_var', 'env_value', 'attr', 'expected'), + [ + pytest.param( + 'ACTOR_STARTED_AT', + '2024-01-01T00:00:00.000Z', + 'started_at', + datetime(2024, 1, 1, tzinfo=UTC), + id='started_at', + ), + pytest.param('APIFY_DEDICATED_CPUS', '1.5', 'dedicated_cpus', 1.5, id='dedicated_cpus'), + pytest.param('ACTOR_TEST_PAY_PER_EVENT', 'true', 'test_pay_per_event', True, id='test_pay_per_event'), + pytest.param( + 'ACTOR_STANDBY_URL', + 'https://standby.apify.com', + 'standby_url', + 'https://standby.apify.com', + id='standby_url', + ), + pytest.param( + 'APIFY_METAMORPH_AFTER_SLEEP_MILLIS', + '1000', + 'metamorph_after_sleep', + timedelta(seconds=1), + id='metamorph_after_sleep', + ), + pytest.param('APIFY_PROXY_PORT', '9000', 'proxy_port', 9000, id='proxy_port'), + pytest.param('ACTOR_WEB_SERVER_PORT', '5000', 'web_server_port', 5000, id='web_server_port'), + pytest.param( + 'APIFY_CHARGED_ACTOR_EVENT_COUNTS', + '{"search": 3}', + 'charged_event_counts', + {'search': 3}, + id='charged_event_counts', + ), + pytest.param('CRAWLEE_PURGE_ON_START', 'false', 'purge_on_start', False, id='purge_on_start'), + ], +) +def test_typed_env_var_non_empty_value_is_still_parsed( + monkeypatch: pytest.MonkeyPatch, env_var: str, env_value: str, attr: str, expected: object +) -> None: + """Only '' counts as unset - a populated typed env var must still reach the field.""" + monkeypatch.setenv(env_var, env_value) + config = ApifyConfiguration() + assert getattr(config, attr) == expected + + +def test_empty_env_var_falls_through_to_legacy_alias(monkeypatch: pytest.MonkeyPatch) -> None: + """An empty value under one alias must not shadow a populated legacy alias of the same field.""" + monkeypatch.setenv('ACTOR_WEB_SERVER_PORT', '') + monkeypatch.setenv('APIFY_CONTAINER_PORT', '9999') + monkeypatch.setenv('ACTOR_ID', '') + monkeypatch.setenv('APIFY_ACTOR_ID', 'legacy-actor-id') + config = ApifyConfiguration() + assert config.web_server_port == 9999 + assert config.actor_id == 'legacy-actor-id' + + +def test_explicitly_passed_empty_string_is_kept(monkeypatch: pytest.MonkeyPatch) -> None: + """Only env vars treat '' as unset - a value passed to the constructor is taken as given.""" + monkeypatch.delenv('ACTOR_INPUT_KEY', raising=False) + config = ApifyConfiguration(input_key='', token='') + assert config.input_key == '' + assert config.token == '' diff --git a/uv.lock b/uv.lock index 4b1767b5..0f5a6a5c 100644 --- a/uv.lock +++ b/uv.lock @@ -46,6 +46,7 @@ dependencies = [ { name = "lazy-object-proxy" }, { name = "more-itertools" }, { name = "pydantic" }, + { name = "pydantic-settings" }, { name = "typing-extensions" }, { name = "websockets" }, { name = "yarl" }, @@ -89,6 +90,7 @@ requires-dist = [ { name = "lazy-object-proxy", specifier = ">=1.11.0" }, { name = "more-itertools", specifier = ">=10.2.0" }, { name = "pydantic", specifier = ">=2.11.0" }, + { name = "pydantic-settings", specifier = ">=2.12.0" }, { name = "scrapy", marker = "extra == 'scrapy'", specifier = ">=2.14.0" }, { name = "typing-extensions", specifier = ">=4.4.0" }, { name = "websockets", specifier = ">=14.0" },