Review comments

Author: Pijukatel

src/apify/_actor.py

@@ -221,7 +221,7 @@ def _finalize_implicit_configuration(self) -> Configuration:
         return self.configuration
 
     def _finalize_implicit_local_storage_client(self) -> StorageClient:
-        """Set implicit local storage client in the actor and return it.
+        """Set implicit local storage client in the Actor and return it.
 
         Changing Actor or service_locator storage client after this method was run is not possible.
         """
@@ -863,7 +863,7 @@ async def start(
         return ActorRun.model_validate(api_result)
 
     def _get_remaining_time(self) -> timedelta | None:
-        """Get time remaining from the actor timeout. Returns `None` if not on an Apify platform."""
+        """Get time remaining from the Actor timeout. Returns `None` if not on an Apify platform."""
         if self.is_at_home() and self.configuration.timeout_at:
             return self.configuration.timeout_at - datetime.now(tz=timezone.utc)
 
@@ -1077,7 +1077,7 @@ async def metamorph(
         if not custom_after_sleep:
             custom_after_sleep = self.configuration.metamorph_after_sleep
 
-        # If is_at_home() is True, config.actor_run_id is always set
+        # If is_at_home() is True, configuration.actor_run_id is always set
         if not self.configuration.actor_run_id:
             raise RuntimeError('actor_run_id cannot be None when running on the Apify platform.')
 
@@ -1229,7 +1229,7 @@ async def create_proxy_configuration(
         self,
         *,
         actor_proxy_input: dict
-        | None = None,  # this is the raw proxy input from the actor run input, it is not spread or snake_cased in here
+        | None = None,  # this is the raw proxy input from the Actor run input, it is not spread or snake_cased in here
         password: str | None = None,
         groups: list[str] | None = None,
         country_code: str | None = None,

src/apify/storage_clients/_apify/_dataset_client.py

@@ -13,7 +13,7 @@
 from crawlee.storage_clients.models import DatasetItemsListPage, DatasetMetadata
 from crawlee.storages import Dataset
 
-from apify.storage_clients._apify._utils import Alias
+from ._utils import AliasResolver
 
 if TYPE_CHECKING:
     from collections.abc import AsyncIterator
@@ -129,7 +129,7 @@ async def open(
 
         if alias:
             # Check if there is pre-existing alias mapping in the default KVS.
-            async with Alias(storage_type=Dataset, alias=alias, configuration=configuration) as _alias:
+            async with AliasResolver(storage_type=Dataset, alias=alias, configuration=configuration) as _alias:
                 id = await _alias.resolve_id()
 
                 # There was no pre-existing alias in the mapping.

src/apify/storage_clients/_apify/_key_value_store_client.py

@@ -13,7 +13,7 @@
 from crawlee.storages import KeyValueStore
 
 from ._models import ApifyKeyValueStoreMetadata, KeyValueStoreListKeysPage
-from ._utils import Alias
+from ._utils import AliasResolver
 from apify._crypto import create_hmac_signature
 
 if TYPE_CHECKING:
@@ -120,7 +120,7 @@ async def open(
 
         if alias:
             # Check if there is pre-existing alias mapping in the default KVS.
-            async with Alias(storage_type=KeyValueStore, alias=alias, configuration=configuration) as _alias:
+            async with AliasResolver(storage_type=KeyValueStore, alias=alias, configuration=configuration) as _alias:
                 id = await _alias.resolve_id()
 
                 # There was no pre-existing alias in the mapping.

src/apify/storage_clients/_apify/_request_queue_client.py

@@ -19,7 +19,7 @@
 from crawlee.storages import RequestQueue
 
 from ._models import CachedRequest, ProlongRequestLockResponse, RequestQueueHead
-from ._utils import Alias
+from ._utils import AliasResolver
 from apify import Request
 
 if TYPE_CHECKING:
@@ -195,7 +195,7 @@ async def open(
 
         if alias:
             # Check if there is pre-existing alias mapping in the default KVS.
-            async with Alias(storage_type=RequestQueue, alias=alias, configuration=configuration) as _alias:
+            async with AliasResolver(storage_type=RequestQueue, alias=alias, configuration=configuration) as _alias:
                 id = await _alias.resolve_id()
 
                 # There was no pre-existing alias in the mapping.

src/apify/storage_clients/_apify/_storage_client.py

@@ -9,7 +9,7 @@
 from ._dataset_client import ApifyDatasetClient
 from ._key_value_store_client import ApifyKeyValueStoreClient
 from ._request_queue_client import ApifyRequestQueueClient
-from ._utils import Alias
+from ._utils import hash_api_base_url_and_token
 from apify._configuration import Configuration as ApifyConfiguration
 from apify._utils import docs_group
 
@@ -31,7 +31,7 @@ class ApifyStorageClient(StorageClient):
     @override
     def get_additional_cache_key(self, configuration: CrawleeConfiguration) -> Hashable:
         if isinstance(configuration, ApifyConfiguration):
-            return Alias.get_additional_cache_key(configuration)
+            return hash_api_base_url_and_token(configuration)
 
         config_class = type(configuration)
         raise TypeError(

src/apify/storage_clients/_apify/_utils.py

@@ -20,7 +20,7 @@
 logger = getLogger(__name__)
 
 
-class Alias:
+class AliasResolver:
     """Class for handling aliases.
 
     The purpose of this is class is to ensure that alias storages are created with correct id. This is achieved by using
@@ -42,9 +42,9 @@ def __init__(
     ) -> None:
         self._storage_type = storage_type
         self._alias = alias
-        self._additional_cache_key = self.get_additional_cache_key(configuration)
+        self._additional_cache_key = hash_api_base_url_and_token(configuration)
 
-    async def __aenter__(self) -> Alias:
+    async def __aenter__(self) -> AliasResolver:
         """Context manager to prevent race condition in alias creation."""
         lock = await self._get_alias_init_lock()
         await lock.acquire()
@@ -60,26 +60,26 @@ async def __aexit__(
     async def _get_alias_init_lock(cls) -> Lock:
         """Get lock for controlling the creation of the alias storages.
 
-        The lock is shared for all instances of the Alias class.
+        The lock is shared for all instances of the AliasResolver class.
         It is created in async method to ensure that some event loop is already running.
         """
         if cls._alias_init_lock is None:
             cls._alias_init_lock = Lock()
         return cls._alias_init_lock
 
     @classmethod
-    async def get_alias_map(cls) -> dict[str, str]:
+    async def _get_alias_map(cls) -> dict[str, str]:
         """Get the aliases and storage ids mapping from the default kvs.
 
-        Mapping is loaded from kvs only once and is shared for all instances of the Alias class.
+        Mapping is loaded from kvs only once and is shared for all instances of the AliasResolver class.
 
         Returns:
             Map of aliases and storage ids.
         """
         if not cls._alias_map:
-            default_kvs_client = await get_default_kvs_client()
+            default_kvs_client = await _get_default_kvs_client()
 
-            record = await default_kvs_client.get_record(cls.ALIAS_MAPPING_KEY)
+            record = await default_kvs_client.get_record(cls._ALIAS_MAPPING_KEY)
 
             # get_record can return {key: ..., value: ..., content_type: ...}
             if isinstance(record, dict):
@@ -92,24 +92,14 @@ async def get_alias_map(cls) -> dict[str, str]:
 
         return cls._alias_map
 
-    @classmethod
-    def get_additional_cache_key(cls, configuration: Configuration) -> str:
-        """Get additional cache key based on configuration.
-
-        Use only api_public_base_url and token as the relevant for differentiating storages.
-        """
-        if configuration.api_public_base_url is None or configuration.token is None:
-            raise ValueError("'Configuration.api_public_base_url' and 'Configuration.token' must be set.")
-        return compute_short_hash(f'{configuration.api_public_base_url}{configuration.token}'.encode())
-
     @property
-    def storage_key(self) -> str:
+    def _storage_key(self) -> str:
         """Get a unique storage key used for storing the alias in the mapping."""
-        return self.ALIAS_STORAGE_KEY_SEPARATOR.join(
+        return self._ALIAS_STORAGE_KEY_SEPARATOR.join(
             [
-                self.storage_type.__name__,
-                self.alias,
-                self.additional_cache_key,
+                self._storage_type.__name__,
+                self._alias,
+                self._additional_cache_key,
             ]
         )
 
@@ -121,38 +111,38 @@ async def resolve_id(self) -> str | None:
         Returns:
             Storage id if it exists, None otherwise.
         """
-        return (await self.get_alias_map()).get(self.storage_key, None)
+        return (await self._get_alias_map()).get(self._storage_key, None)
 
     async def store_mapping(self, storage_id: str) -> None:
         """Add alias and related storage id to the mapping in default kvs and local in-memory mapping."""
         # Update in-memory mapping
-        (await self.get_alias_map())[self.storage_key] = storage_id
+        (await self._get_alias_map())[self._storage_key] = storage_id
         if not Configuration.get_global_configuration().is_at_home:
             logging.getLogger(__name__).warning(
-                'Alias storage limited retention is only supported on Apify platform. Storage is not exported.'
+                'AliasResolver storage limited retention is only supported on Apify platform. Storage is not exported.'
             )
             return
 
-        default_kvs_client = await get_default_kvs_client()
+        default_kvs_client = await _get_default_kvs_client()
         await default_kvs_client.get()
 
         try:
-            record = await default_kvs_client.get_record(self.ALIAS_MAPPING_KEY)
+            record = await default_kvs_client.get_record(self._ALIAS_MAPPING_KEY)
 
             # get_record can return {key: ..., value: ..., content_type: ...}
             if isinstance(record, dict) and 'value' in record:
                 record = record['value']
 
             # Update or create the record with the new alias mapping
             if isinstance(record, dict):
-                record[self.storage_key] = storage_id
+                record[self._storage_key] = storage_id
             else:
-                record = {self.storage_key: storage_id}
+                record = {self._storage_key: storage_id}
 
             # Store the mapping back in the KVS.
-            await default_kvs_client.set_record(self.ALIAS_MAPPING_KEY, record)
+            await default_kvs_client.set_record(self._ALIAS_MAPPING_KEY, record)
         except Exception as exc:
-            logger.warning(f'Error storing alias mapping for {self.alias}: {exc}')
+            logger.warning(f'Error storing alias mapping for {self._alias}: {exc}')
 
 
 async def _get_default_kvs_client() -> KeyValueStoreClientAsync:
@@ -168,3 +158,10 @@ async def _get_default_kvs_client() -> KeyValueStoreClientAsync:
     )
 
     return apify_client_async.key_value_store(key_value_store_id=configuration.default_key_value_store_id)
+
+
+def hash_api_base_url_and_token(configuration: Configuration) -> str:
+    """Hash configuration.api_public_base_url and configuration.token in deterministic way."""
+    if configuration.api_public_base_url is None or configuration.token is None:
+        raise ValueError("'Configuration.api_public_base_url' and 'Configuration.token' must be set.")
+    return compute_short_hash(f'{configuration.api_public_base_url}{configuration.token}'.encode())

tests/integration/conftest.py

@@ -20,7 +20,7 @@
 from ._utils import generate_unique_resource_name
 from apify import Actor
 from apify._models import ActorRun
-from apify.storage_clients._apify._utils import Alias
+from apify.storage_clients._apify._utils import AliasResolver
 
 if TYPE_CHECKING:
     from collections.abc import AsyncGenerator, Awaitable, Callable, Coroutine, Iterator, Mapping
@@ -61,9 +61,9 @@ def _prepare_test_env() -> None:
         service_locator._storage_client = None
         service_locator.storage_instance_manager.clear_cache()
 
-        # Reset the Alias class state.
-        Alias._alias_map = {}
-        Alias._alias_init_lock = None
+        # Reset the AliasResolver class state.
+        AliasResolver._alias_map = {}
+        AliasResolver._alias_init_lock = None
 
         # Verify that the test environment was set up correctly.
         assert os.environ.get(ApifyEnvVars.LOCAL_STORAGE_DIR) == str(tmp_path)

tests/unit/conftest.py

@@ -17,7 +17,7 @@
 
 import apify._actor
 import apify.log
-from apify.storage_clients._apify._utils import Alias
+from apify.storage_clients._apify._utils import AliasResolver
 
 if TYPE_CHECKING:
     from collections.abc import Callable, Iterator
@@ -73,9 +73,9 @@ def _prepare_test_env() -> None:
         service_locator._storage_client = None
         service_locator.storage_instance_manager.clear_cache()
 
-        # Reset the Alias class state.
-        Alias._alias_map = {}
-        Alias._alias_init_lock = None
+        # Reset the AliasResolver class state.
+        AliasResolver._alias_map = {}
+        AliasResolver._alias_init_lock = None
 
         # Verify that the test environment was set up correctly.
         assert os.environ.get(ApifyEnvVars.LOCAL_STORAGE_DIR) == str(tmp_path)