-
Notifications
You must be signed in to change notification settings - Fork 68
LCORE-1137: token history table #939
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,191 @@ | ||
| """Class with implementation of storage for token usage history. | ||
| One table named `token_usage` is used to store statistic about token usage | ||
| history. Input and output token count are stored for each triple (user_id, | ||
| provider, model). This triple is also used as a primary key to this table. | ||
| """ | ||
|
|
||
| import sqlite3 | ||
| from datetime import datetime | ||
| import psycopg2 | ||
|
|
||
| from log import get_logger | ||
|
|
||
| from quota.connect_pg import connect_pg | ||
| from quota.connect_sqlite import connect_sqlite | ||
| from quota.sql import ( | ||
| CREATE_TOKEN_USAGE_TABLE, | ||
| CONSUME_TOKENS_FOR_USER_PG, | ||
| CONSUME_TOKENS_FOR_USER_SQLITE, | ||
| ) | ||
|
|
||
| from models.config import QuotaHandlersConfiguration | ||
| from utils.connection_decorator import connection | ||
|
|
||
| logger = get_logger(__name__) | ||
|
|
||
|
|
||
| class TokenUsageHistory: | ||
| """Class with implementation of storage for token usage history.""" | ||
|
|
||
| def __init__(self, configuration: QuotaHandlersConfiguration) -> None: | ||
| """Initialize token usage history storage. | ||
| Initialize TokenUsageHistory with the provided configuration and | ||
| establish a database connection. | ||
| Stores SQLite and PostgreSQL connection settings for later reconnection | ||
| attempts, initializes the internal connection state, and opens the | ||
| database connection. | ||
| Parameters: | ||
| configuration (QuotaHandlersConfiguration): Configuration | ||
| containing `sqlite` and `postgres` connection settings. | ||
| """ | ||
| # store the configuration, it will be used | ||
| # by reconnection logic later, if needed | ||
| self.sqlite_connection_config = configuration.sqlite | ||
| self.postgres_connection_config = configuration.postgres | ||
| self.connection = None | ||
|
Comment on lines
+47
to
+49
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. 🛠️ Refactor suggestion | 🟠 Major Add type annotations for class attributes. The coding guidelines require complete type annotations for all class attributes. Add type hints for As per coding guidelines, all class attributes must include type annotations. 🔎 Proposed fix+from typing import Any
+from models.config import SQLiteDatabaseConfiguration, PostgreSQLDatabaseConfiguration
+
class TokenUsageHistory:
"""Class with implementation of storage for token usage history."""
def __init__(self, configuration: QuotaHandlersConfiguration) -> None:
"""Initialize token usage history storage.
Initialize TokenUsageHistory with the provided configuration and
establish a database connection.
Stores SQLite and PostgreSQL connection settings for later reconnection
attempts, initializes the internal connection state, and opens the
database connection.
Parameters:
configuration (QuotaHandlersConfiguration): Configuration
containing `sqlite` and `postgres` connection settings.
"""
# store the configuration, it will be used
# by reconnection logic later, if needed
- self.sqlite_connection_config = configuration.sqlite
- self.postgres_connection_config = configuration.postgres
- self.connection = None
+ self.sqlite_connection_config: SQLiteDatabaseConfiguration | None = configuration.sqlite
+ self.postgres_connection_config: PostgreSQLDatabaseConfiguration | None = configuration.postgres
+ self.connection: Any | None = None
🤖 Prompt for AI Agents |
||
|
|
||
| # initialize connection to DB | ||
| self.connect() | ||
|
|
||
| # pylint: disable=W0201 | ||
| def connect(self) -> None: | ||
| """Initialize connection to database. | ||
| Establish a database connection for token usage history and ensure required tables exist. | ||
| Selects PostgreSQL if its configuration is present, otherwise uses | ||
| SQLite; initializes the token_usage table, enables autocommit on the | ||
| connection, and ensures the connection is closed and the exception is | ||
| re-raised if table initialization fails. | ||
| Raises: | ||
| ValueError: If neither PostgreSQL nor SQLite configuration is provided. | ||
| """ | ||
| logger.info("Initializing connection to quota usage history database") | ||
| if self.postgres_connection_config is not None: | ||
| self.connection = connect_pg(self.postgres_connection_config) | ||
| if self.sqlite_connection_config is not None: | ||
| self.connection = connect_sqlite(self.sqlite_connection_config) | ||
| if self.connection is None: | ||
| return | ||
|
Comment on lines
+69
to
+74
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. SQLite configuration silently overrides PostgreSQL when both are present. If both 🔎 Proposed fix using elif logger.info("Initializing connection to quota usage history database")
if self.postgres_connection_config is not None:
self.connection = connect_pg(self.postgres_connection_config)
- if self.sqlite_connection_config is not None:
+ elif self.sqlite_connection_config is not None:
self.connection = connect_sqlite(self.sqlite_connection_config)
if self.connection is None:
returnNote: The same pattern exists in 🤖 Prompt for AI Agents
Comment on lines
+65
to
+74
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. Docstring claims ValueError is raised, but implementation returns silently. The docstring states "Raises: ValueError: If neither PostgreSQL nor SQLite configuration is provided," but the code returns early without raising any exception when 🔎 Proposed fix to match docstring if self.postgres_connection_config is not None:
self.connection = connect_pg(self.postgres_connection_config)
if self.sqlite_connection_config is not None:
self.connection = connect_sqlite(self.sqlite_connection_config)
if self.connection is None:
- return
+ raise ValueError("Neither PostgreSQL nor SQLite configuration is provided")Or update the docstring to reflect the current behavior: - Raises:
- ValueError: If neither PostgreSQL nor SQLite configuration is provided.
+ Note:
+ Returns early without establishing a connection if neither PostgreSQL
+ nor SQLite configuration is provided.🤖 Prompt for AI Agents |
||
|
|
||
| try: | ||
| self._initialize_tables() | ||
| except Exception as e: | ||
| self.connection.close() | ||
| logger.exception("Error initializing quota usage history database:\n%s", e) | ||
| raise | ||
|
|
||
| self.connection.autocommit = True | ||
|
|
||
| @connection | ||
| def consume_tokens( # pylint: disable=too-many-arguments,too-many-positional-arguments | ||
| self, | ||
| user_id: str, | ||
| provider: str, | ||
| model: str, | ||
| input_tokens: int, | ||
| output_tokens: int, | ||
| ) -> None: | ||
| """Consume tokens by given user. | ||
| Record token usage for a specific user/provider/model triple in persistent storage. | ||
| Parameters: | ||
| user_id (str): Identifier of the user whose token usage will be updated. | ||
| provider (str): Provider name associated with the usage (e.g., "openai"). | ||
| model (str): Model name associated with the usage (e.g., "gpt-4"). | ||
| input_tokens (int): Number of input tokens to add to the stored usage. | ||
| output_tokens (int): Number of output tokens to add to the stored usage. | ||
| Raises: | ||
| ValueError: If no database backend configuration (Postgres or SQLite) is available. | ||
| """ | ||
| logger.info( | ||
| "Token usage for user %s, provider %s and model %s changed by %d, %d tokens", | ||
| user_id, | ||
| provider, | ||
| model, | ||
| input_tokens, | ||
| output_tokens, | ||
| ) | ||
| query_statement: str = "" | ||
| if self.postgres_connection_config is not None: | ||
| query_statement = CONSUME_TOKENS_FOR_USER_PG | ||
| if self.sqlite_connection_config is not None: | ||
| query_statement = CONSUME_TOKENS_FOR_USER_SQLITE | ||
|
Comment on lines
+117
to
+120
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. Same precedence issue as in The query statement selection has the same issue where SQLite config overwrites PostgreSQL config if both are present. Use 🔎 Proposed fix query_statement: str = ""
if self.postgres_connection_config is not None:
query_statement = CONSUME_TOKENS_FOR_USER_PG
- if self.sqlite_connection_config is not None:
+ elif self.sqlite_connection_config is not None:
query_statement = CONSUME_TOKENS_FOR_USER_SQLITE🤖 Prompt for AI Agents |
||
|
|
||
| # check if the connection was established | ||
| if self.connection is None: | ||
| logger.warning("Not connected, need to reconnect later") | ||
| return | ||
|
|
||
| # timestamp to be used | ||
| updated_at = datetime.now() | ||
|
|
||
| # it is not possible to use context manager there, because SQLite does | ||
| # not support it | ||
| cursor = self.connection.cursor() | ||
| cursor.execute( | ||
| query_statement, | ||
| { | ||
| "user_id": user_id, | ||
| "provider": provider, | ||
| "model": model, | ||
| "input_tokens": input_tokens, | ||
| "output_tokens": output_tokens, | ||
| "updated_at": updated_at, | ||
| }, | ||
| ) | ||
| cursor.close() | ||
|
|
||
| def connected(self) -> bool: | ||
| """Check if connection to quota usage history database is alive. | ||
| Verify that the storage connection for token usage history is alive. | ||
| Returns: | ||
| `true` if the database connection is present and responds to a | ||
| simple query, `false` otherwise. | ||
| """ | ||
| if self.connection is None: | ||
| logger.warning("Not connected, need to reconnect later") | ||
| return False | ||
| cursor = None | ||
| try: | ||
| cursor = self.connection.cursor() | ||
| cursor.execute("SELECT 1") | ||
| logger.info("Connection to storage is ok") | ||
| return True | ||
| except (psycopg2.OperationalError, sqlite3.Error) as e: | ||
| logger.error("Disconnected from storage: %s", e) | ||
| return False | ||
| finally: | ||
| if cursor is not None: | ||
| try: | ||
| cursor.close() | ||
| except Exception: # pylint: disable=broad-exception-caught | ||
| logger.warning("Unable to close cursor") | ||
|
|
||
| def _initialize_tables(self) -> None: | ||
| """Initialize tables used by quota limiter. | ||
| Ensure the token_usage table exists in the configured database. | ||
| Creates the table required to store per-user token usage history (input/output tokens per | ||
| user_id, provider, model) and commits the change to the database. | ||
| """ | ||
| # check if the connection was established | ||
| if self.connection is None: | ||
| logger.warning("Not connected, need to reconnect later") | ||
| return | ||
|
|
||
| logger.info("Initializing tables for token usage history") | ||
| cursor = self.connection.cursor() | ||
| cursor.execute(CREATE_TOKEN_USAGE_TABLE) | ||
| cursor.close() | ||
| self.connection.commit() | ||
Uh oh!
There was an error while loading. Please reload this page.