-
Notifications
You must be signed in to change notification settings - Fork 58
LCORE-298: Conversation cache factory #567
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| """Various cache implementations.""" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| """Abstract class that is parent for all cache implementations.""" | ||
|
|
||
| from abc import ABC, abstractmethod | ||
|
|
||
| from models.cache_entry import CacheEntry | ||
| from utils.suid import check_suid | ||
|
|
||
|
|
||
| class Cache(ABC): | ||
| """Abstract class that is parent for all cache implementations. | ||
|
|
||
| Cache entries are identified by compound key that consists of | ||
| user ID and conversation ID. Application logic must ensure that | ||
| user will be able to store and retrieve values that have the | ||
| correct user ID only. This means that user won't be able to | ||
| read or modify other users conversations. | ||
| """ | ||
|
|
||
| # separator between parts of compond key | ||
| COMPOUND_KEY_SEPARATOR = ":" | ||
|
|
||
| @staticmethod | ||
| def _check_user_id(user_id: str, skip_user_id_check: bool) -> None: | ||
| """Check if given user ID is valid.""" | ||
| if skip_user_id_check: | ||
| return | ||
| if not check_suid(user_id): | ||
| raise ValueError(f"Invalid user ID {user_id}") | ||
|
|
||
| @staticmethod | ||
| def _check_conversation_id(conversation_id: str) -> None: | ||
| """Check if given conversation ID is a valid UUID (including optional dashes).""" | ||
| if not check_suid(conversation_id): | ||
| raise ValueError(f"Invalid conversation ID {conversation_id}") | ||
|
|
||
| @staticmethod | ||
| def construct_key( | ||
| user_id: str, conversation_id: str, skip_user_id_check: bool | ||
| ) -> str: | ||
| """Construct key to cache.""" | ||
| Cache._check_user_id(user_id, skip_user_id_check) | ||
| Cache._check_conversation_id(conversation_id) | ||
| return f"{user_id}{Cache.COMPOUND_KEY_SEPARATOR}{conversation_id}" | ||
|
|
||
| @abstractmethod | ||
| def get( | ||
| self, user_id: str, conversation_id: str, skip_user_id_check: bool | ||
| ) -> list[CacheEntry]: | ||
| """Abstract method to retrieve a value from the cache. | ||
|
|
||
| Args: | ||
| user_id: User identification. | ||
| conversation_id: Conversation ID unique for given user. | ||
| skip_user_id_check: Skip user_id suid check. | ||
|
|
||
| Returns: | ||
| The value (CacheEntry(s)) associated with the key, or None if not found. | ||
| """ | ||
|
|
||
| @abstractmethod | ||
| def insert_or_append( | ||
| self, | ||
| user_id: str, | ||
| conversation_id: str, | ||
| cache_entry: CacheEntry, | ||
| skip_user_id_check: bool, | ||
| ) -> None: | ||
| """Abstract method to store a value in the cache. | ||
|
|
||
| Args: | ||
| user_id: User identification. | ||
| conversation_id: Conversation ID unique for given user. | ||
| cache_entry: The value to store. | ||
| skip_user_id_check: Skip user_id suid check. | ||
| """ | ||
|
|
||
| @abstractmethod | ||
| def delete( | ||
| self, user_id: str, conversation_id: str, skip_user_id_check: bool | ||
| ) -> bool: | ||
| """Delete all entries for a given conversation. | ||
|
|
||
| Args: | ||
| user_id: User identification. | ||
| conversation_id: Conversation ID unique for given user. | ||
| skip_user_id_check: Skip user_id suid check. | ||
|
|
||
| Returns: | ||
| bool: True if entries were deleted, False if key wasn't found. | ||
| """ | ||
|
|
||
| @abstractmethod | ||
| def list(self, user_id: str, skip_user_id_check: bool) -> list[str]: | ||
| """List all conversations for a given user_id. | ||
|
|
||
| Args: | ||
| user_id: User identification. | ||
| skip_user_id_check: Skip user_id suid check. | ||
|
|
||
| Returns: | ||
| A list of conversation ids from the cache | ||
| """ | ||
|
|
||
| @abstractmethod | ||
| def ready(self) -> bool: | ||
| """Check if the cache is ready. | ||
|
|
||
| Returns: | ||
| True if the cache is ready, False otherwise. | ||
| """ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| """Any exception that can occur during cache operations.""" | ||
|
|
||
|
|
||
| class CacheError(Exception): | ||
| """Any exception that can occur during cache operations.""" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| """Cache factory class.""" | ||
|
|
||
| import constants | ||
| from models.config import ConversationCacheConfiguration | ||
| from cache.cache import Cache | ||
| from log import get_logger | ||
|
|
||
| logger = get_logger("cache.cache_factory") | ||
|
|
||
|
|
||
| # pylint: disable=R0903 | ||
| class CacheFactory: | ||
| """Cache factory class.""" | ||
|
|
||
| @staticmethod | ||
| def conversation_cache(config: ConversationCacheConfiguration) -> Cache | None: | ||
| """Create an instance of Cache based on loaded configuration. | ||
| Returns: | ||
| An instance of `Cache` (either `PostgresCache` or `InMemoryCache`). | ||
| """ | ||
| logger.info("Creating cache instance of type %s", config.type) | ||
| match config.type: | ||
| case constants.CACHE_TYPE_MEMORY: | ||
| return None | ||
| case constants.CACHE_TYPE_SQLITE: | ||
| return None | ||
| case constants.CACHE_TYPE_POSTGRES: | ||
| return None | ||
| case _: | ||
| raise ValueError( | ||
| f"Invalid cache type: {config.type}. " | ||
| f"Use '{constants.CACHE_TYPE_POSTGRES}' '{constants.CACHE_TYPE_SQLITE}' or " | ||
| f"'{constants.CACHE_TYPE_MEMORY}' options." | ||
| ) | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -19,9 +19,12 @@ | |||||||||||||||||||||||||||||||||||||||||
| AuthenticationConfiguration, | ||||||||||||||||||||||||||||||||||||||||||
| InferenceConfiguration, | ||||||||||||||||||||||||||||||||||||||||||
| DatabaseConfiguration, | ||||||||||||||||||||||||||||||||||||||||||
| ConversationCache, | ||||||||||||||||||||||||||||||||||||||||||
| ConversationCacheConfiguration, | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| from cache.cache import Cache | ||||||||||||||||||||||||||||||||||||||||||
| from cache.cache_factory import CacheFactory | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| logger = logging.getLogger(__name__) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -44,6 +47,7 @@ def __new__(cls, *args: Any, **kwargs: Any) -> "AppConfig": | |||||||||||||||||||||||||||||||||||||||||
| def __init__(self) -> None: | ||||||||||||||||||||||||||||||||||||||||||
| """Initialize the class instance.""" | ||||||||||||||||||||||||||||||||||||||||||
| self._configuration: Optional[Configuration] = None | ||||||||||||||||||||||||||||||||||||||||||
| self._conversation_cache: Optional[Cache] = None | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| def load_configuration(self, filename: str) -> None: | ||||||||||||||||||||||||||||||||||||||||||
| """Load configuration from YAML file.""" | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -126,7 +130,7 @@ def inference(self) -> InferenceConfiguration: | |||||||||||||||||||||||||||||||||||||||||
| return self._configuration.inference | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| @property | ||||||||||||||||||||||||||||||||||||||||||
| def conversation_cache(self) -> ConversationCache: | ||||||||||||||||||||||||||||||||||||||||||
| def conversation_cache_configuration(self) -> ConversationCacheConfiguration: | ||||||||||||||||||||||||||||||||||||||||||
| """Return conversation cache configuration.""" | ||||||||||||||||||||||||||||||||||||||||||
| if self._configuration is None: | ||||||||||||||||||||||||||||||||||||||||||
| raise LogicError("logic error: configuration is not loaded") | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -139,5 +143,14 @@ def database_configuration(self) -> DatabaseConfiguration: | |||||||||||||||||||||||||||||||||||||||||
| raise LogicError("logic error: configuration is not loaded") | ||||||||||||||||||||||||||||||||||||||||||
| return self._configuration.database | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| @property | ||||||||||||||||||||||||||||||||||||||||||
| def conversation_cache(self) -> Cache | None: | ||||||||||||||||||||||||||||||||||||||||||
| """Return the conversation cache.""" | ||||||||||||||||||||||||||||||||||||||||||
| if self._conversation_cache is None and self._configuration is not None: | ||||||||||||||||||||||||||||||||||||||||||
| self._conversation_cache = CacheFactory.conversation_cache( | ||||||||||||||||||||||||||||||||||||||||||
| self._configuration.conversation_cache | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
| return self._conversation_cache | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+146
to
+153
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Guard for unloaded config and None cache config before calling factory Avoids passing None to the factory and keeps behavior consistent with other accessors. @property
def conversation_cache(self) -> Cache | None:
"""Return the conversation cache."""
- if self._conversation_cache is None and self._configuration is not None:
- self._conversation_cache = CacheFactory.conversation_cache(
- self._configuration.conversation_cache
- )
- return self._conversation_cache
+ if self._configuration is None:
+ raise LogicError("logic error: configuration is not loaded")
+ if self._conversation_cache is None:
+ cfg = self._configuration.conversation_cache
+ if cfg is None or cfg.type is None:
+ self._conversation_cache = None
+ else:
+ self._conversation_cache = CacheFactory.conversation_cache(cfg)
+ return self._conversation_cache📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| configuration: AppConfig = AppConfig() | ||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| """Model for conversation history cache entry.""" | ||
|
|
||
| from pydantic import BaseModel | ||
|
|
||
|
|
||
| class CacheEntry(BaseModel): | ||
| """Model representing a cache entry. | ||
|
|
||
| Attributes: | ||
| query: The query string | ||
| response: The response string | ||
| provider: Provider identification | ||
| model: Model identification | ||
| """ | ||
|
|
||
| query: str | ||
| response: str | ||
| provider: str | ||
| model: str |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle None/disabled cache config and fix docstring/options text
Factory currently assumes config is non‑None and will crash when configuration.conversation_cache is None or when type is None (a valid state per validator). Also docstring mentions only Postgres/InMemory while SQLite is supported by the type.
📝 Committable suggestion
🤖 Prompt for AI Agents