Review comments

Author: Pijukatel

src/apify/_actor.py

@@ -25,7 +25,6 @@
     EventPersistStateData,
     EventSystemInfoData,
 )
-from crawlee.storage_clients import FileSystemStorageClient
 
 from apify._charging import ChargeResult, ChargingManager, ChargingManagerImplementation
 from apify._configuration import Configuration
@@ -258,24 +257,17 @@ def _storage_client(self) -> SmartApifyStorageClient:
             return implicit_storage_client
 
         # User set something in the service locator.
-        storage_client = service_locator.get_storage_client()
-        if isinstance(storage_client, SmartApifyStorageClient):
+        explicit_storage_client = service_locator.get_storage_client()
+        if isinstance(explicit_storage_client, SmartApifyStorageClient):
             # The client was manually set to the right type in the service locator. This is the explicit way.
-            return storage_client
-
-        if isinstance(storage_client, ApifyStorageClient):
-            # The cloud storage client was manually set in the service locator.
-            return SmartApifyStorageClient(cloud_storage_client=storage_client)
-
-        # The local storage client was manually set in the service locator
-        if type(storage_client) is FileSystemStorageClient:
-            self.log.warning(
-                f'Using {FileSystemStorageClient.__module__}.{FileSystemStorageClient.__name__} in Actor context is not'
-                f' recommended and can lead to problems with reading the input file. Use '
-                f'`apify.storage_clients.FileSystemStorageClient` instead.'
-            )
+            return explicit_storage_client
 
-        return SmartApifyStorageClient(cloud_storage_client=ApifyStorageClient(), local_storage_client=storage_client)
+        raise RuntimeError(
+            'The storage client in the service locator has to be instance of SmartApifyStorageClient. If you want to '
+            'set the storage client manually you have to call '
+            '`service_locator.set_storage_client(SmartApifyStorageClient(...))` before entering Actor context or '
+            'awaiting `Actor.init`.'
+        )
 
     async def init(self) -> None:
         """Initialize the Actor instance.

src/apify/storage_clients/_apify/_utils.py

@@ -107,7 +107,7 @@ async def store_mapping(self, storage_id: str) -> None:
         # Update in-memory mapping
         (await self._get_alias_map())[self._storage_key] = storage_id
         if not Configuration.get_global_configuration().is_at_home:
-            logging.getLogger(__name__).warning(
+            logging.getLogger(__name__).debug(
                 'AliasResolver storage limited retention is only supported on Apify platform. Storage is not exported.'
             )
             return

src/apify/storage_clients/_smart_apify/_storage_client.py

@@ -1,6 +1,5 @@
 from __future__ import annotations
 
-from functools import cached_property
 from typing import TYPE_CHECKING
 
 from typing_extensions import override
@@ -56,7 +55,7 @@ def get_suitable_storage_client(self, *, force_cloud: bool = False) -> StorageCl
         Args:
             force_cloud: If True, return `cloud_storage_client`.
         """
-        if self._is_at_home:
+        if ApifyConfiguration.get_global_configuration().is_at_home:
             return self._cloud_storage_client
 
         configuration = ApifyConfiguration.get_global_configuration()
@@ -72,7 +71,7 @@ def get_suitable_storage_client(self, *, force_cloud: bool = False) -> StorageCl
 
     @override
     def get_additional_cache_key(self, configuration: CrawleeConfiguration) -> Hashable:
-        if self._is_at_home:
+        if ApifyConfiguration.get_global_configuration().is_at_home:
             if isinstance(configuration, ApifyConfiguration):
                 return self._cloud_storage_client.get_additional_cache_key(configuration)
             raise TypeError('Expecting ApifyConfiguration')
@@ -117,8 +116,3 @@ async def create_rq_client(
         return await self.get_suitable_storage_client().create_rq_client(
             id=id, name=id, alias=alias, configuration=configuration
         )
-
-    @cached_property
-    def _is_at_home(self) -> bool:
-        configuration = ApifyConfiguration.get_global_configuration()
-        return configuration.is_at_home

tests/integration/test_apify_storages.py

@@ -67,7 +67,6 @@ async def test_aliases_not_stored_on_platform_when_local(
 ) -> None:
     """Test that default Apify storage used locally is not persisting aliases to Apify based default KVS."""
     service_locator.set_configuration(Configuration(token=apify_token))
-    service_locator.set_storage_client(ApifyStorageClient())
     async with Actor(configure_logging=False):
         await storage_type.open(alias='test')
         default_kvs = await Actor.open_key_value_store(force_cloud=True)
@@ -111,23 +110,12 @@ async def test_actor_full_explicit_storage_init_same_client(apify_token: str) ->
 async def test_actor_partial_explicit_cloud_storage_init(apify_token: str) -> None:
     service_locator.set_configuration(Configuration(token=apify_token))
     service_locator.set_storage_client(ApifyStorageClient(request_queue_access='shared'))
-    async with Actor:
-        # If service locator was already set with ApifyStorageClient, the actor will use it as cloud_storage_client of
-        # SmartApifyStorageClient
-        assert await Actor.open_dataset() is not await Actor.open_dataset(force_cloud=True)
-        assert await Actor.open_key_value_store() is not await Actor.open_key_value_store(force_cloud=True)
-        assert await Actor.open_request_queue() is not await Actor.open_request_queue(force_cloud=True)
-
-
-async def test_actor_partial_explicit_local_storage_init(apify_token: str) -> None:
-    service_locator.set_configuration(Configuration(token=apify_token))
-    service_locator.set_storage_client(MemoryStorageClient())
-    async with Actor:
-        # If service locator was already set with non-ApifyStorageClient, the actor will use it as local_storage_client
-        # of SmartApifyStorageClient
-        assert await Actor.open_dataset() is not await Actor.open_dataset(force_cloud=True)
-        assert await Actor.open_key_value_store() is not await Actor.open_key_value_store(force_cloud=True)
-        assert await Actor.open_request_queue() is not await Actor.open_request_queue(force_cloud=True)
+    with pytest.raises(
+        RuntimeError, match=r'^The storage client in the service locator has to be instance of SmartApifyStorageClient'
+    ):
+        async with Actor:
+            # If service locator was explicitly set to something different than SmartApifyStorageClient, raise an error.
+            ...
 
 
 async def test_actor_implicit_storage_init(apify_token: str) -> None:

tests/unit/actor/test_configuration.py

@@ -7,7 +7,6 @@
 from crawlee.configuration import Configuration as CrawleeConfiguration
 from crawlee.crawlers import BasicCrawler
 from crawlee.errors import ServiceConflictError
-from crawlee.storage_clients import FileSystemStorageClient
 
 from apify import Actor
 from apify import Configuration as ApifyConfiguration
@@ -242,16 +241,3 @@ def test_apify_configuration_is_always_used(caplog: pytest.LogCaptureFixture) ->
         'It is recommended to set `apify.Configuration` explicitly as early as possible by using '
         'service_locator.set_configuration'
     ) in caplog.messages
-
-
-async def test_file_system_storage_client_warning(caplog: pytest.LogCaptureFixture) -> None:
-    service_locator.set_storage_client(FileSystemStorageClient())
-    caplog.set_level('WARNING')
-    async with Actor():
-        ...
-
-    assert (
-        'Using crawlee.storage_clients._file_system._storage_client.FileSystemStorageClient in Actor context is not '
-        'recommended and can lead to problems with reading the input file. Use '
-        '`apify.storage_clients.FileSystemStorageClient` instead.'
-    ) in caplog.messages