feat(BA-4702): Implement prometheus query preset repository layer#9470
feat(BA-4702): Implement prometheus query preset repository layer#9470HyeockJinKim merged 5 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements the repository layer for the prometheus query preset system as specified in issue #9365 (BA-4702). It provides the foundational data access layer that enables CRUD operations for managing prometheus query presets, including support for filtering, searching, and ordering. The implementation follows established repository patterns in the codebase, using resilience policies and the base repository abstractions.
Changes:
- Added data types, modifiers, and list result classes for prometheus query presets
- Implemented repository with CRUD operations (create, get_by_id, search, update, delete)
- Added query conditions and orders for flexible searching and filtering
- Created comprehensive unit tests covering CRUD operations and query options
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ai/backend/manager/data/prometheus_query_preset/types.py | Added frozen dataclass for preset data, modifier for updates, and list result type |
| src/ai/backend/manager/data/prometheus_query_preset/init.py | Exported new data types |
| src/ai/backend/common/exception.py | Added PrometheusQueryPresetNotFound exception and PROMETHEUS_QUERY_PRESET error domain |
| src/ai/backend/common/metrics/metric.py | Added PROMETHEUS_QUERY_PRESET_REPOSITORY layer type for metrics |
| src/ai/backend/manager/repositories/prometheus_query_preset/repository.py | Main repository class with resilience policies and public CRUD methods |
| src/ai/backend/manager/repositories/prometheus_query_preset/db_source/db_source.py | Database operations implementation with session management |
| src/ai/backend/manager/repositories/prometheus_query_preset/creators.py | Creator spec for preset creation |
| src/ai/backend/manager/repositories/prometheus_query_preset/updaters.py | Updater spec for preset updates |
| src/ai/backend/manager/repositories/prometheus_query_preset/repositories.py | Repositories wrapper following established pattern |
| src/ai/backend/manager/repositories/prometheus_query_preset/options.py | Query conditions and orders for search operations |
| src/ai/backend/manager/repositories/prometheus_query_preset/init.py | Package exports |
| tests/unit/manager/repositories/prometheus_query_preset/test_prometheus_query_preset_repository.py | Unit tests for CRUD operations |
| tests/unit/manager/repositories/prometheus_query_preset/test_prometheus_query_preset_options.py | Unit tests for query conditions and orders |
| tests/unit/manager/repositories/prometheus_query_preset/BUILD | Build configuration for tests |
| changes/9470.feature.md | Changelog entry |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...unit/manager/repositories/prometheus_query_preset/test_prometheus_query_preset_repository.py
Show resolved
Hide resolved
src/ai/backend/manager/repositories/prometheus_query_preset/db_source/db_source.py
Show resolved
Hide resolved
tests/unit/manager/repositories/prometheus_query_preset/test_prometheus_query_preset_options.py
Outdated
Show resolved
Hide resolved
13e5250 to
f6d2e51
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 17 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self.filter_labels.update_dict(to_update, "filter_labels") | ||
| self.group_labels.update_dict(to_update, "group_labels") |
There was a problem hiding this comment.
PrometheusQueryPresetModifier.fields_to_update() populates keys "filter_labels" and "group_labels", but PrometheusQueryPresetRow does not have such columns (they are nested under the JSONB "options" column). If this modifier is used to build an UPDATE statement (as other *Modifier types are), it will generate invalid column updates. Consider either removing this modifier until it is wired correctly, or have it emit an "options" update (or provide a dedicated updater spec that correctly merges/updates PresetOptions).
| self.filter_labels.update_dict(to_update, "filter_labels") | |
| self.group_labels.update_dict(to_update, "group_labels") | |
| # NOTE: filter_labels and group_labels are stored under a JSONB "options" | |
| # column in the underlying row type, so they must be handled by a dedicated | |
| # updater/merger rather than as top-level columns here. |
| @prometheus_query_preset_repository_resilience.apply() | ||
| async def get_by_id(self, preset_id: UUID) -> PrometheusQueryPresetData: | ||
| """Retrieves a prometheus query preset by ID.""" | ||
| return await self._db_source.get_by_id(preset_id) |
There was a problem hiding this comment.
The repository layer currently exposes create/update/delete/get_by_id/search, but the referenced issue scope/acceptance criteria also calls for get_by_name. If get_by_name is required for the service/API layers, please add it to both PrometheusQueryPresetRepository and PrometheusQueryPresetDBSource (with a matching unit test) to avoid forcing callers to emulate it via a search.
| test_case: SearchContext, | ||
| ) -> None: | ||
| assert isinstance( | ||
| test_case.case, ConditionCase |
There was a problem hiding this comment.
This isinstance() assert is only here for type-narrowing, but when it fails it won’t provide a helpful message. Consider matching the pattern used in test_search_with_order (line ~301) by adding an explicit assertion message (or using typing.cast) so failures are easier to diagnose.
| test_case.case, ConditionCase | |
| test_case.case, ConditionCase | |
| ), ( | |
| f"Expected test_case.case to be ConditionCase in test_search_by_condition, " | |
| f"got {type(test_case.case)!r}" |
resolves #9365 (BA-4702)
Checklist: (if applicable)
ai.backend.testdocsdirectory