refactor!: Make Actor initialization stricter and more predictable

pyproject.toml

@@ -36,7 +36,7 @@ keywords = [
 dependencies = [
     "apify-client>=2.0.0,<3.0.0",
     "apify-shared>=2.0.0,<3.0.0",
-    "crawlee==1.0.0rc1",
+    "crawlee @ git+https://github.com/apify/crawlee-python.git@storage-clients-and-configurations-2",
     "cachetools>=5.5.0",
     "cryptography>=42.0.0",
     "impit>=0.5.3",

src/apify/_configuration.py

@@ -8,6 +8,7 @@
 from pydantic import AliasChoices, BeforeValidator, Field, model_validator
 from typing_extensions import Self, deprecated
 
+from crawlee import service_locator
 from crawlee._utils.models import timedelta_ms
 from crawlee._utils.urls import validate_http_url
 from crawlee.configuration import Configuration as CrawleeConfiguration
@@ -424,11 +425,35 @@ def disable_browser_sandbox_on_platform(self) -> Self:
     def get_global_configuration(cls) -> Configuration:
         """Retrieve the global instance of the configuration.
 
-        Mostly for the backwards compatibility. It is recommended to use the `service_locator.get_configuration()`
-        instead.
+        This method ensures that ApifyConfigration is returned, even if CrawleeConfiguration was set in the
+        service locator.
         """
-        return cls()
+        global_configuration = service_locator.get_configuration()
 
+        if isinstance(global_configuration, Configuration):
+            # If Apify configuration was already stored in service locator, return it.
+            return global_configuration
 
-# Monkey-patch the base class so that it works with the extended configuration
-CrawleeConfiguration.get_global_configuration = Configuration.get_global_configuration  # type: ignore[method-assign]
+        return cls.from_configuration(global_configuration)
+
+    @classmethod
+    def from_configuration(cls, configuration: CrawleeConfiguration) -> Configuration:
+        """Create Apify Configuration from existing Crawlee Configuration.
+
+        Args:
+            configuration: The existing Crawlee Configuration.
+
+        Returns:
+            The created Apify Configuration.
+        """
+        apify_configuration = cls()
+
+        # Ensure the returned configuration is of type Apify Configuration.
+        # Most likely crawlee configuration was already set. Create Apify configuration from it.
+        # Due to known Pydantic issue https://github.com/pydantic/pydantic/issues/9516, creating new instance of
+        # Configuration from existing one in situation where environment can have some fields set by alias is very
+        # unpredictable. Use the stable workaround.
+        for name in configuration.model_fields:
+            setattr(apify_configuration, name, getattr(configuration, name))
+
+        return apify_configuration

tests/integration/actor_source_base/requirements.txt

@@ -1,4 +1,4 @@
 # The test fixture will put the Apify SDK wheel path on the next line
 APIFY_SDK_WHEEL_PLACEHOLDER
 uvicorn[standard]
-crawlee[parsel]==1.0.0rc1
+crawlee[parsel] @ git+https://github.com/apify/crawlee-python.git@storage-clients-and-configurations-2

tests/integration/conftest.py

@@ -58,18 +58,10 @@ def _prepare_test_env() -> None:
         service_locator._configuration = None
         service_locator._event_manager = None
         service_locator._storage_client = None
-        service_locator._storage_instance_manager = None
-
-        # Reset the retrieval flags.
-        service_locator._configuration_was_retrieved = False
-        service_locator._event_manager_was_retrieved = False
-        service_locator._storage_client_was_retrieved = False
+        service_locator.storage_instance_manager.clear_cache()
 
         # Verify that the test environment was set up correctly.
         assert os.environ.get(ApifyEnvVars.LOCAL_STORAGE_DIR) == str(tmp_path)
-        assert service_locator._configuration_was_retrieved is False
-        assert service_locator._storage_client_was_retrieved is False
-        assert service_locator._event_manager_was_retrieved is False
 
     return _prepare_test_env
 

tests/unit/actor/test_actor_lifecycle.py

@@ -150,6 +150,9 @@ async def handler(websocket: websockets.asyncio.server.ServerConnection) -> None
         port: int = ws_server.sockets[0].getsockname()[1]  # type: ignore[index]
         monkeypatch.setenv(ActorEnvVars.EVENTS_WEBSOCKET_URL, f'ws://localhost:{port}')
 
+        # Make sure there is a charging manager that can be mocked
+        Actor._get_charging_manager_implementation()
+
         mock_run_client = Mock()
         mock_run_client.run.return_value.get = AsyncMock(
             side_effect=lambda: {

tests/unit/actor/test_configuration.py

@@ -1,6 +1,15 @@
+from pathlib import Path
+
 import pytest
 
-from apify import Configuration
+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
+
+from apify import Actor
+from apify import Configuration as ApifyConfiguration
 
 
 @pytest.mark.parametrize(
@@ -16,6 +25,181 @@ def test_disable_browser_sandbox(
     *, is_at_home: bool, disable_browser_sandbox_in: bool, disable_browser_sandbox_out: bool
 ) -> None:
     assert (
-        Configuration(is_at_home=is_at_home, disable_browser_sandbox=disable_browser_sandbox_in).disable_browser_sandbox
+        ApifyConfiguration(
+            is_at_home=is_at_home, disable_browser_sandbox=disable_browser_sandbox_in
+        ).disable_browser_sandbox
         == disable_browser_sandbox_out
     )
+
+
+def test_apify_configuration_is_always_used() -> None:
+    """Set Crawlee Configuration in Actor and verify that Apify Configuration is used."""
+    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))
+
+    assert Actor.config.max_used_cpu_ratio == max_used_cpu_ratio
+    assert isinstance(Actor.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
+
+    assert Actor.config is not crawlee_config
+    assert isinstance(Actor.config, ApifyConfiguration)
+    # Make sure the Crawlee Configuration was used to create ApifyConfiguration in Actor
+    assert Actor.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 setting configuration in service locator after actor was 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 setting configuration in service locator after actor was 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 = BasicCrawler()
+
+    assert Actor.config is service_locator.get_configuration() is crawler._service_locator.get_configuration()
+
+
+async def test_crawlers_own_configuration(tmp_path: Path) -> None:
+    """Test that crawlers can use own configurations without crashing."""
+    config_actor = ApifyConfiguration()
+    dir_1 = tmp_path / 'dir_1'
+    dir_2 = tmp_path / 'dir_2'
+    config_crawler_1 = ApifyConfiguration()
+    config_actor.storage_dir = str(dir_1)
+    config_crawler_2 = ApifyConfiguration()
+    config_crawler_2.storage_dir = str(dir_2)
+
+    async with Actor(configuration=config_actor):
+
+        async def request_handler(context: BasicCrawlingContext) -> None:
+            Actor.log.info(f'Processing: {context.request.url}')
+
+        crawler_1 = BasicCrawler(configuration=config_crawler_1, request_handler=request_handler)
+        crawler_2 = BasicCrawler(configuration=config_crawler_2, request_handler=request_handler)
+        await crawler_1.add_requests([Request.from_url(url='http://example.com/1')])
+        await crawler_2.add_requests(
+            [Request.from_url(url='http://example.com/2'), Request.from_url(url='http://example.com/3')]
+        )
+
+        await crawler_1.run()
+        await crawler_2.run()
+
+    assert service_locator.get_configuration() is config_actor
+    assert crawler_1._service_locator.get_configuration() is config_crawler_1
+    assert crawler_2._service_locator.get_configuration() is config_crawler_2
+
+    assert crawler_1.statistics.state.requests_total == 1
+    assert crawler_2.statistics.state.requests_total == 2
+
+
+async def test_crawler_global_configuration() -> None:
+    """Test that crawler and Actor use service_locator based configuration unless explicit configuration
+    was passed to them."""
+    config_global = ApifyConfiguration()
+    service_locator.set_configuration(config_global)
+
+    async with Actor():
+        crawler = BasicCrawler()
+
+    assert service_locator.get_configuration() is config_global
+    assert crawler._service_locator.get_configuration() is config_global
+
+
+async def test_crawler_uses_implicit_apify_config() -> None:
+    """Test that Actor is using implicit ApifyConfiguration in Actor context."""
+    async with Actor:
+        assert isinstance(Actor.config, ApifyConfiguration)
+
+
+async def test_storage_retrieved_is_different_with_different_config(tmp_path: Path) -> None:
+    """Test that retrieving storage depends on used configuration."""
+    dir_1 = tmp_path / 'dir_1'
+    dir_2 = tmp_path / 'dir_2'
+    config_actor = ApifyConfiguration()
+    config_actor.storage_dir = str(dir_1)
+    config_crawler = ApifyConfiguration()
+    config_crawler.storage_dir = str(dir_2)
+
+    async with Actor(configuration=config_actor):
+        actor_kvs = await Actor.open_key_value_store()
+        crawler = BasicCrawler(configuration=config_crawler)
+        crawler_kvs = await crawler.get_key_value_store()
+
+    assert actor_kvs is not crawler_kvs
+
+
+async def test_storage_retrieved_is_same_with_equivalent_config() -> None:
+    """Test that retrieving storage depends on used configuration. If two equivalent configuration(even if they are
+    different instances) are used it returns same storage."""
+    config_actor = ApifyConfiguration()
+    apify_crawler = ApifyConfiguration()
+
+    async with Actor(configuration=config_actor):
+        actor_kvs = await Actor.open_key_value_store()
+        crawler = BasicCrawler(configuration=apify_crawler)
+        crawler_kvs = await crawler.get_key_value_store()
+
+    assert actor_kvs is crawler_kvs
+
+
+async def test_storage_retrieved_is_same_with_same_config() -> None:
+    """Test that retrieving storage is same if same configuration is used."""
+    async with Actor():
+        actor_kvs = await Actor.open_key_value_store()
+        crawler = BasicCrawler()
+        crawler_kvs = await crawler.get_key_value_store()
+
+    assert actor_kvs is crawler_kvs

tests/unit/conftest.py

@@ -70,18 +70,10 @@ def _prepare_test_env() -> None:
         service_locator._configuration = None
         service_locator._event_manager = None
         service_locator._storage_client = None
-        service_locator._storage_instance_manager = None
-
-        # Reset the retrieval flags.
-        service_locator._configuration_was_retrieved = False
-        service_locator._event_manager_was_retrieved = False
-        service_locator._storage_client_was_retrieved = False
+        service_locator.storage_instance_manager.clear_cache()
 
         # Verify that the test environment was set up correctly.
         assert os.environ.get(ApifyEnvVars.LOCAL_STORAGE_DIR) == str(tmp_path)
-        assert service_locator._configuration_was_retrieved is False
-        assert service_locator._storage_client_was_retrieved is False
-        assert service_locator._event_manager_was_retrieved is False
 
     return _prepare_test_env
 

tests/unit/events/test_apify_event_manager.py

@@ -24,9 +24,8 @@
 
 async def test_lifecycle_local(caplog: pytest.LogCaptureFixture) -> None:
     caplog.set_level(logging.DEBUG, logger='apify')
-    config = Configuration.get_global_configuration()
 
-    async with ApifyEventManager(config):
+    async with ApifyEventManager(Configuration()):
         pass
 
     assert len(caplog.records) == 1

tests/unit/test_proxy_configuration.py

@@ -532,9 +532,9 @@ async def test_initialize_when_status_page_unavailable(
 
     await proxy_configuration.initialize()
 
-    assert len(caplog.records) == 1
+    assert len(caplog.records) == 2
     assert caplog.records[0].levelname == 'WARNING'
-    assert 'Apify Proxy access check timed out' in caplog.records[0].message
+    assert 'Apify Proxy access check timed out' in caplog.records[1].message
 
 
 async def test_initialize_with_non_apify_proxy(