Add many configuration based tests

Author: Pijukatel

src/apify/_actor.py

@@ -13,6 +13,7 @@
 
 from apify_client import ApifyClientAsync
 from apify_shared.consts import ActorEnvVars, ActorExitCodes, ApifyEnvVars
+from crawlee.errors import ServiceConflictError
 from crawlee.events import (
     Event,
     EventAbortingData,
@@ -118,9 +119,12 @@ def __init__(
         self._exit_process = self._get_default_exit_process() if exit_process is None else exit_process
         self._is_exiting = False
 
-        if configuration:
-            service_locator.set_configuration(configuration)
-        self._configuration = service_locator.get_configuration()
+        # Actor state when this method is being executed is unpredictable.
+        # Actor can be initialized by lazy object proxy or by user directly, or by both.
+        # Until `init` method is run, this state of uncertainty remains. This is the reason why any setting done here in
+        # `__init__` method should not be considered final.
+
+        self._configuration = configuration
         self._configure_logging = configure_logging
         self._apify_client = self.new_client()
 
@@ -131,17 +135,17 @@ def __init__(
         # Set the event manager based on whether the Actor is running on the platform or locally.
         self._event_manager = (
             ApifyEventManager(
-                configuration=self._configuration,
-                persist_state_interval=self._configuration.persist_state_interval,
+                configuration=self.config,
+                persist_state_interval=self.config.persist_state_interval,
             )
             if self.is_at_home()
             else LocalEventManager(
-                system_info_interval=self._configuration.system_info_interval,
-                persist_state_interval=self._configuration.persist_state_interval,
+                system_info_interval=self.config.system_info_interval,
+                persist_state_interval=self.config.persist_state_interval,
             )
         )
 
-        self._charging_manager = ChargingManagerImplementation(self._configuration, self._apify_client)
+        self._charging_manager = ChargingManagerImplementation(self.config, self._apify_client)
 
         self._is_initialized = False
 
@@ -204,12 +208,18 @@ def apify_client(self) -> ApifyClientAsync:
     @property
     def configuration(self) -> Configuration:
         """The Configuration instance the Actor instance uses."""
-        return self._configuration
+        return self.config
 
     @property
     def config(self) -> Configuration:
         """The Configuration instance the Actor instance uses."""
-        return self._configuration
+        if self._configuration:
+            return self._configuration
+        self.log.debug(
+            'Implicit configuration used.'
+            "It's recommended to explicitly set the configuration to avoid unexpected behavior."
+        )
+        return Configuration()
 
     @property
     def event_manager(self) -> EventManager:
@@ -251,6 +261,21 @@ async def init(self) -> None:
         This method should be called immediately before performing any additional Actor actions, and it should be
         called only once.
         """
+        if self._configuration:
+            # Set explicitly the configuration in the service locator
+            service_locator.set_configuration(self.configuration)
+        else:
+            try:
+                # Set implicit default Apify configuration, unless configuration was already set.
+                service_locator.set_configuration(self.configuration)
+            except ServiceConflictError:
+                self.log.info(
+                    'Configuration in service locator was set explicitly before Actor. '
+                    'Using the existing configuration.'
+                )
+            # Use the configuration from the service locator
+            self._configuration = service_locator.get_configuration()
+
         if self._is_initialized:
             raise RuntimeError('The Actor was already initialized!')
 
@@ -268,7 +293,6 @@ async def init(self) -> None:
             service_locator.set_storage_client(self._cloud_storage_client)
 
         service_locator.set_event_manager(self.event_manager)
-        service_locator.set_configuration(self.configuration)
 
         # The logging configuration has to be called after all service_locator set methods.
         if self._configure_logging:
@@ -386,8 +410,8 @@ def new_client(
                 (increases exponentially from this value).
             timeout: The socket timeout of the HTTP requests sent to the Apify API.
         """
-        token = token or self._configuration.token
-        api_url = api_url or self._configuration.api_base_url
+        token = token or self.config.token
+        api_url = api_url or self.config.api_base_url
         return ApifyClientAsync(
             token=token,
             api_url=api_url,
@@ -547,9 +571,9 @@ async def get_input(self) -> Any:
         """Get the Actor input value from the default key-value store associated with the current Actor run."""
         self._raise_if_not_initialized()
 
-        input_value = await self.get_value(self._configuration.input_key)
-        input_secrets_private_key = self._configuration.input_secrets_private_key_file
-        input_secrets_key_passphrase = self._configuration.input_secrets_private_key_passphrase
+        input_value = await self.get_value(self.config.input_key)
+        input_secrets_private_key = self.config.input_secrets_private_key_file
+        input_secrets_key_passphrase = self.config.input_secrets_private_key_passphrase
         if input_secrets_private_key and input_secrets_key_passphrase:
             private_key = load_private_key(
                 input_secrets_private_key,
@@ -686,7 +710,7 @@ def off(self, event_name: Event, listener: Callable | None = None) -> None:
 
     def is_at_home(self) -> bool:
         """Return `True` when the Actor is running on the Apify platform, and `False` otherwise (e.g. local run)."""
-        return self._configuration.is_at_home
+        return self.config.is_at_home
 
     def get_env(self) -> dict:
         """Return a dictionary with information parsed from all the `APIFY_XXX` environment variables.
@@ -712,7 +736,7 @@ def get_env(self) -> dict:
                 aliases = [field_name]
 
             for alias in aliases:
-                config[alias] = getattr(self._configuration, field_name)
+                config[alias] = getattr(self.config, field_name)
 
         env_vars = {env_var.value.lower(): env_var.name.lower() for env_var in [*ActorEnvVars, *ApifyEnvVars]}
         return {option_name: config[env_var] for env_var, option_name in env_vars.items() if env_var in config}
@@ -1000,13 +1024,13 @@ async def metamorph(
             return
 
         if not custom_after_sleep:
-            custom_after_sleep = self._configuration.metamorph_after_sleep
+            custom_after_sleep = self.config.metamorph_after_sleep
 
         # If is_at_home() is True, config.actor_run_id is always set
-        if not self._configuration.actor_run_id:
+        if not self.config.actor_run_id:
             raise RuntimeError('actor_run_id cannot be None when running on the Apify platform.')
 
-        await self._apify_client.run(self._configuration.actor_run_id).metamorph(
+        await self._apify_client.run(self.config.actor_run_id).metamorph(
             target_actor_id=target_actor_id,
             run_input=run_input,
             target_actor_build=target_actor_build,
@@ -1043,7 +1067,7 @@ async def reboot(
         _ActorType._is_rebooting = True
 
         if not custom_after_sleep:
-            custom_after_sleep = self._configuration.metamorph_after_sleep
+            custom_after_sleep = self.config.metamorph_after_sleep
 
         # Call all the listeners for the PERSIST_STATE and MIGRATING events, and wait for them to finish.
         # PERSIST_STATE listeners are called to allow the Actor to persist its state before the reboot.
@@ -1063,10 +1087,10 @@ async def reboot(
             *[listener(EventMigratingData()) for listener in migrating_listeners],
         )
 
-        if not self._configuration.actor_run_id:
+        if not self.config.actor_run_id:
             raise RuntimeError('actor_run_id cannot be None when running on the Apify platform.')
 
-        await self._apify_client.run(self._configuration.actor_run_id).reboot()
+        await self._apify_client.run(self.config.actor_run_id).reboot()
 
         if custom_after_sleep:
             await asyncio.sleep(custom_after_sleep.total_seconds())
@@ -1105,11 +1129,11 @@ async def add_webhook(
             return
 
         # If is_at_home() is True, config.actor_run_id is always set
-        if not self._configuration.actor_run_id:
+        if not self.config.actor_run_id:
             raise RuntimeError('actor_run_id cannot be None when running on the Apify platform.')
 
         await self._apify_client.webhooks().create(
-            actor_run_id=self._configuration.actor_run_id,
+            actor_run_id=self.config.actor_run_id,
             event_types=webhook.event_types,
             request_url=webhook.request_url,
             payload_template=webhook.payload_template,
@@ -1141,10 +1165,10 @@ async def set_status_message(
             return None
 
         # If is_at_home() is True, config.actor_run_id is always set
-        if not self._configuration.actor_run_id:
+        if not self.config.actor_run_id:
             raise RuntimeError('actor_run_id cannot be None when running on the Apify platform.')
 
-        api_result = await self._apify_client.run(self._configuration.actor_run_id).update(
+        api_result = await self._apify_client.run(self.config.actor_run_id).update(
             status_message=status_message, is_status_message_terminal=is_terminal
         )
 
@@ -1199,7 +1223,7 @@ async def create_proxy_configuration(
             country_code=country_code,
             proxy_urls=proxy_urls,
             new_url_function=new_url_function,
-            _actor_config=self._configuration,
+            _actor_config=self.config,
             _apify_client=self._apify_client,
         )
 

src/apify/_configuration.py

@@ -440,13 +440,21 @@ class ApifyServiceLocator(ServiceLocator):
     def get_configuration(self) -> Configuration:
         # ApifyServiceLocator can store any children of Crawlee Configuration, but in Apify context it is desired to
         # return Apify Configuration.
+        if isinstance(self._configuration, Configuration):
+            # If Apify configuration was already stored in service locator, return it.
+            return self._configuration
+
         stored_configuration = super().get_configuration()
+
         # Ensure the returned configuration is of type Apify Configuration.
+        # Most likely crawlee configuration was already set. Create Apify configuration from it.
+        # Env vars will have lower priority than the stored configuration.
         model_dump = stored_configuration.model_dump()
         # The configuration will read env variables first and overridden with stored_configuration
         _config = Configuration()
         for key in model_dump:
-            setattr(_config, key, model_dump[key])
+            if model_dump[key]:
+                setattr(_config, key, model_dump[key])
         return _config
 
 

tests/unit/actor/test_configuration.py

@@ -1,7 +1,10 @@
 import pytest
 
 from crawlee.configuration import Configuration as CrawleeConfiguration
+from crawlee.crawlers import BasicCrawler
+from crawlee.errors import ServiceConflictError
 
+from apify import Actor
 from apify import Configuration as ApifyConfiguration
 from apify._configuration import service_locator
 
@@ -28,10 +31,108 @@ def test_disable_browser_sandbox(
 
 def test_apify_configuration_is_always_used() -> None:
     """Set Crawlee Configuration in service_locator and verify that Apify Configuration is returned."""
-    # Some value to verify
-    max_used_cpu_ratio = 0.123456
+    max_used_cpu_ratio = 0.123456  # Some unique value to verify configuration
     service_locator.set_configuration(CrawleeConfiguration(max_used_cpu_ratio=max_used_cpu_ratio))
 
     returned_config = service_locator.get_configuration()
     assert returned_config.max_used_cpu_ratio == max_used_cpu_ratio
     assert isinstance(returned_config, ApifyConfiguration)
+
+
+async def test_existing_apify_config_respected_by_actor() -> None:
+    """Set Apify Configuration in service_locator and verify that Actor respects it."""
+    max_used_cpu_ratio = 0.123456  # Some unique value to verify configuration
+    apify_config = ApifyConfiguration(max_used_cpu_ratio=max_used_cpu_ratio)
+    service_locator.set_configuration(apify_config)
+    async with Actor:
+        pass
+
+    returned_config = service_locator.get_configuration()
+    assert returned_config is apify_config
+
+
+async def test_existing_crawlee_config_respected_by_actor() -> None:
+    """Set Crawlee Configuration in service_locator and verify that Actor respects it."""
+    max_used_cpu_ratio = 0.123456  # Some unique value to verify configuration
+    crawlee_config = CrawleeConfiguration(max_used_cpu_ratio=max_used_cpu_ratio)
+    service_locator.set_configuration(crawlee_config)
+    async with Actor:
+        pass
+
+    returned_config = service_locator.get_configuration()
+    assert returned_config is not crawlee_config
+    assert isinstance(returned_config, ApifyConfiguration)
+    # Make sure the Crawlee Configuration was used to create returned Apify Configuration
+    assert returned_config.max_used_cpu_ratio == max_used_cpu_ratio
+
+
+async def test_existing_apify_config_throws_error_when_set_in_actor() -> None:
+    """Test that passing explicit configuration to actor after service locator configuration was already set,
+    raises exception."""
+    service_locator.set_configuration(ApifyConfiguration())
+    with pytest.raises(ServiceConflictError):
+        async with Actor(configuration=ApifyConfiguration()):
+            pass
+
+
+async def test_setting_config_after_actor_raises_exception() -> None:
+    """Test that passing setting configuration in service locator after actor wa created raises an exception."""
+    async with Actor():
+        with pytest.raises(ServiceConflictError):
+            service_locator.set_configuration(ApifyConfiguration())
+
+
+async def test_actor_using_input_configuration() -> None:
+    """Test that passing setting configuration in service locator after actor wa created raises an exception."""
+    apify_config = ApifyConfiguration()
+    async with Actor(configuration=apify_config):
+        pass
+
+    assert service_locator.get_configuration() is apify_config
+
+
+async def test_crawler_implicit_configuration_through_actor() -> None:
+    """Test that crawler uses Actor configuration unless explicit configuration was passed to it."""
+    apify_config = ApifyConfiguration()
+    async with Actor(configuration=apify_config):
+        crawler = BasicCrawler()
+
+    assert crawler._service_locator.get_configuration() is apify_config
+    assert service_locator.get_configuration() is apify_config
+
+
+async def test_crawler_implicit_configuration() -> None:
+    """Test that crawler and Actor use implicit service_locator based configuration unless explicit configuration
+    was passed to them."""
+    async with Actor():
+        crawler_1 = BasicCrawler()
+
+    assert service_locator.get_configuration() is crawler_1._service_locator.get_configuration()
+
+
+async def test_crawlers_own_configuration() -> None:
+    """Test that crawlers can use own configurations without crashing."""
+    config_actor = ApifyConfiguration()
+    apify_crawler_1 = ApifyConfiguration()
+    apify_crawler_2 = ApifyConfiguration()
+
+    async with Actor(configuration=config_actor):
+        crawler_1 = BasicCrawler(configuration=apify_crawler_1)
+        crawler_2 = BasicCrawler(configuration=apify_crawler_2)
+
+    assert service_locator.get_configuration() is config_actor
+    assert crawler_1._service_locator.get_configuration() is apify_crawler_1
+    assert crawler_2._service_locator.get_configuration() is apify_crawler_2
+
+
+async def test_crawler_global_configuration() -> None:
+    """Test that crawler and Actor use explicit service_locator based configuration unless explicit configuration
+    was passed to them."""
+    config_global = ApifyConfiguration()
+    service_locator.set_configuration(config_global)
+
+    async with Actor():
+        crawler_1 = BasicCrawler()
+
+    assert service_locator.get_configuration() is config_global
+    assert crawler_1._service_locator.get_configuration() is config_global