fix(BA-5470): Add missing update, execute commands and admin CLI module for v2 prometheus-query-preset#10606
Open
seedspirit wants to merge 9 commits intomainfrom
Open
fix(BA-5470): Add missing update, execute commands and admin CLI module for v2 prometheus-query-preset#10606seedspirit wants to merge 9 commits intomainfrom
seedspirit wants to merge 9 commits intomainfrom
Conversation
Add execute() to V2PrometheusQueryPresetClient that POSTs to
/v2/prometheus-query-presets/{preset_id}/execute with
ExecuteQueryDefinitionInput and returns ExecuteQueryDefinitionPayload.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…T handler
Add execute handler method that calls adapter.execute_preset() and converts
QueryDefinitionResultInfo to ExecuteQueryDefinitionPayload. Register route
POST /{preset_id}/execute with superadmin_required middleware. Also fix
ExecuteQueryDefinitionInput.time_range to use QueryTimeRangeInputDTO (datetime)
instead of client QueryTimeRange (str) for type consistency with the adapter.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add execute command with --start, --end, --step, --label, --group-labels, and --time-window options. Includes _parse_label_filters helper for parsing key=value label format into MetricLabelEntry objects. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add admin commands (search, create, update, delete) under `./bai admin prometheus-query-preset` and register the LazyGroup in the admin __init__.py. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…e_label_filters Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
github-actions
bot
added
comp:manager
Contributor
There was a problem hiding this comment.
Pull request overview
Completes the v2 Prometheus query preset surface area by adding preset execution support across the REST API, SDK, and CLIs, plus introducing an admin-focused CLI module.
Changes:
- Added REST v2 route + handler for executing a Prometheus query preset.
- Added v2 SDK
execute()plus CLIupdate/executecommands for prometheus-query-preset. - Introduced an admin CLI module (lazy-registered) with search/create/update/delete commands; aligned
time_rangeDTO type.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ai/backend/manager/api/rest/v2/prometheus_query_preset/registry.py | Registers the new execute endpoint for presets. |
| src/ai/backend/manager/api/rest/v2/prometheus_query_preset/handler.py | Implements the REST handler for preset execution and response shaping. |
| src/ai/backend/common/dto/manager/v2/prometheus_query_preset/request.py | Updates ExecuteQueryDefinitionInput.time_range to use QueryTimeRangeInputDTO. |
| src/ai/backend/client/v2/domains_v2/prometheus_query_preset.py | Adds SDK method to call the new execute endpoint. |
| src/ai/backend/client/cli/v2/prometheus_query_preset/commands.py | Adds user-facing update and execute commands and label/time-range parsing. |
| src/ai/backend/client/cli/v2/admin/prometheus_query_preset.py | Adds admin CLI commands for managing query presets. |
| src/ai/backend/client/cli/v2/admin/init.py | Lazy-registers the new admin CLI group. |
| changes/10606.feature.md | Adds changelog entry for the new CLI/SDK features. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
24
to
28
| reg.add("POST", "/search", handler.admin_search, middlewares=[superadmin_required]) | ||
| reg.add("GET", "/{preset_id}", handler.get, middlewares=[superadmin_required]) | ||
| reg.add("PATCH", "/{preset_id}", handler.update, middlewares=[superadmin_required]) | ||
| reg.add("POST", "/{preset_id}/execute", handler.execute, middlewares=[superadmin_required]) | ||
| reg.add("POST", "/delete", handler.delete, middlewares=[superadmin_required]) |
There was a problem hiding this comment.
The PR description says update/execute are added to the “user-facing v2 prometheus-query-preset module”, but the manager REST routes (including the newly added execute) are guarded by superadmin_required. If these commands are intended for non-superadmins, the REST route middleware needs to be relaxed (or a separate non-admin route should be added). If they are superadmin-only, consider moving these CLI commands under the admin CLI module to match the authorization model.
src/ai/backend/client/cli/v2/prometheus_query_preset/commands.py
Outdated
Show resolved
Hide resolved
src/ai/backend/client/cli/v2/prometheus_query_preset/commands.py
Outdated
Show resolved
Hide resolved
src/ai/backend/client/cli/v2/prometheus_query_preset/commands.py
Outdated
Show resolved
Hide resolved
src/ai/backend/client/cli/v2/prometheus_query_preset/commands.py
Outdated
Show resolved
Hide resolved
src/ai/backend/client/cli/v2/prometheus_query_preset/commands.py
Outdated
Show resolved
Hide resolved
Comment on lines
+142
to
+144
| result = await registry.prometheus_query_preset.update( | ||
| UUID(preset_id), ModifyQueryDefinitionInput(**data) | ||
| ) |
There was a problem hiding this comment.
Same as the user CLI: preset_id is typed as str but is later coerced to UUID(...), which can lead to unhandled tracebacks on invalid input. Use a Click UUID argument type (or explicit click.BadParameter) for consistent, user-friendly validation failures.
…etheus-query-definition The external-facing CLI command name should match the v1 naming convention (query-definition), not the internal package name (query-preset). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
seedspirit
changed the title
seedspirit
changed the title
… validation - Use click.UUID type for all preset_id arguments instead of str+UUID() conversion, providing clean CLI validation errors on invalid input - Add validation that --start, --end, --step must all be provided together or none at all, instead of silently ignoring partial input Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.
Summary
execute()method to v2 SDK and REST handler for prometheus query presetsupdateandexecuteCLI commands to user-facing v2 prometheus-query-preset modulesearch,create,update,deletecommands registered via LazyGroupExecuteQueryDefinitionInput.time_rangetype to useQueryTimeRangeInputDTOfor consistencyTest plan
pants check --changed-since=origin/mainpassespants test --changed-since=origin/mainpasses./bai prometheus-query-preset update <id> <json>works./bai prometheus-query-preset execute <id>works with time range and label options./bai admin prometheus-query-preset searchworks./bai admin prometheus-query-preset create/update/deleteworkResolves BA-5470