feat(BA-2734): Add domain-level app configuration support for frontend#6401
Merged
Conversation
github-actions
Bot
added
size:XL
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR implements a comprehensive configuration management system for frontend applications with support for domain, project, and user-level scopes. The implementation follows a repository pattern with database persistence and Redis/Valkey caching for efficient configuration retrieval.
Key changes include:
- Database schema with
app_configstable supporting hierarchical scope-based configuration (domain → project → user) - Repository pattern implementation with separate DB and cache sources
- Cache invalidation strategy using Redis Sets to track domain-user relationships
- ValkeyCache client wrapper providing direct Glide client access for cache operations
Reviewed Changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/ai/backend/manager/models/alembic/versions/c3b9dacd4f79_*.py |
Alembic migration creating app_configs table with scope-based design |
src/ai/backend/manager/models/app_config.py |
SQLAlchemy model definition for app_configs table |
src/ai/backend/manager/data/app_config/types.py |
Data models for configuration entities and operations |
src/ai/backend/manager/repositories/app_config/db_source/db_source.py |
Database operations layer with config merging logic |
src/ai/backend/manager/repositories/app_config/cache_source/cache_source.py |
Cache operations with domain-level invalidation using Redis Sets |
src/ai/backend/manager/repositories/app_config/repository.py |
Main repository coordinating DB and cache sources |
src/ai/backend/manager/clients/valkey_client/valkey_cache/client.py |
ValkeyCache client wrapper for direct Glide access |
src/ai/backend/manager/repositories/utils.py |
Shared repository utilities including exception suppression |
tests/manager/repositories/app_config/test_app_config.py |
Comprehensive integration tests for repository functionality |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+20
to
+22
| count = 0 | ||
|
|
||
|
|
There was a problem hiding this comment.
Global variable 'count' appears to be debug code that should be removed before merging to production. This creates shared mutable state that will cause issues in multi-threaded/async environments and is not thread-safe.
Suggested change
| count = 0 |
Comment on lines
+61
to
+62
| global count | ||
| count += 1 |
There was a problem hiding this comment.
Debug code using global variable should be removed. This counter is not thread-safe and will produce incorrect results in concurrent environments.
| global count | ||
| count += 1 | ||
| if cached_value: | ||
| log.debug("Cache hit for merged config: {}, hit count: {}", user_id, count) |
There was a problem hiding this comment.
Remove debug logging with global counter. This appears to be temporary debugging code.
Implement scope-based configuration system with database schema, repository pattern, and cache layer for efficient configuration management. - Add app_configs table with scope-based design (DOMAIN/PROJECT/USER) - Implement hierarchical config merging (domain → user override) - Add repository pattern with DB and cache sources - Create ValkeyCache client for manager-level cache operations - Implement efficient domain-level cache invalidation using Redis Sets - Use Batch operations for atomic-like cache updates Database Schema: - app_configs table with scope_type, scope_id, and JSONB extra_config - Unique constraint on (scope_type, scope_id) - Created alembic migration c3b9dacd4f79 Data Models: - AppConfigData: Database row representation - MergedAppConfig: Contains domain_name, user_id, and merged config - AppConfigCreator/Modifier: For create and upsert operations Repository Pattern: - AppConfigDBSource: Handles all database operations - AppConfigCacheSource: Handles Redis/Valkey cache operations - AppConfigRepository: Combines sources with cache-aside pattern - ValkeyCache: Manager-level client providing direct Glide access Cache Strategy: - Cache merged configs per user with 10-minute TTL - Track users per domain using Redis Sets - Domain config changes invalidate all domain users - User config changes invalidate only that user - Use Valkey Batch for efficient multi-operation execution
Add comprehensive tests for AppConfigRepository with real database and Valkey cache. Fix circular import issue by moving AppConfigScopeType to data layer. Tests: - Create domain-level and user-level configurations - Test merged config retrieval with override logic - Test upsert (create/update) operations - Test delete operations - Test cache invalidation on config updates Fixtures: - test_domain_name: Creates domain in DB, returns domain name string - test_user_id: Creates user in DB, returns user UUID string - valkey_stat_client: Real Valkey client using redis container - app_config_repository: Repository with real DB and cache Fixes: - Move AppConfigScopeType from models to data/app_config/types - Prevents circular dependency between models and data layers
…ache invalidation logic
HyeockJinKim
force-pushed
the
feat/add-domain-level-config-db
branch
from
63e59fb to
cfa39b2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implement scope-based configuration system with database schema,
repository pattern, and cache layer for efficient configuration
management.
Database Schema:
Data Models:
Repository Pattern:
Cache Strategy:
resolves #6294 (BA-2734)
Checklist: (if applicable)
ai.backend.testdocsdirectory