Conversation
Marenz
commented
May 16, 2025
- Reset release notes
- Add two new filters to the dispatch list command
Signed-off-by: Mathias L. Baumann <mathias.baumann@frequenz.com>
There was a problem hiding this comment.
Pull Request Overview
This PR resets the release notes and enhances the dispatch list CLI command by adding two new filters.
- Added
--runningand--typeoptions to filter dispatches by status and type. - Updated filtering logic to count and report filtered-out dispatches.
- Reset and stubbed out sections in
RELEASE_NOTES.md.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/frequenz/client/dispatch/main.py | Introduced --running and --type options, applied filtering in the loop, and updated summary output. |
| RELEASE_NOTES.md | Cleared existing notes, added placeholders for Summary and Upgrading, and documented new filters. |
Comments suppressed due to low confidence (1)
src/frequenz/client/dispatch/main.py:241
- No tests cover the new
--runningand--typefilters. Add unit or integration tests to verify that dispatches are correctly included or excluded based on these flags.
async for page in ctx.obj["client"].list(**filters):
| @click.option("--dry-run", type=bool) | ||
| @click.option("--page-size", type=int) | ||
| @click.option("--running", type=bool) | ||
| @click.option("--type", "-T", type=str) |
There was a problem hiding this comment.
Using the option name --type shadows Python’s built-in type and may confuse users; consider renaming the flag (e.g. --dispatch-type) and setting dest='dispatch_type'.
| @click.option("--type", "-T", type=str) | |
| @click.option("--dispatch-type", "-T", type=str, dest="dispatch_type") |
| @click.option("--running", type=bool) | ||
| @click.option("--type", "-T", type=str) |
There was a problem hiding this comment.
Add a help= string for the new options (--running and --type) so users know exactly how to use them in the CLI.
| @click.option("--running", type=bool) | |
| @click.option("--type", "-T", type=str) | |
| @click.option( | |
| "--running", | |
| type=bool, | |
| help="Filter dispatches based on whether they are currently running.", | |
| ) | |
| @click.option( | |
| "--type", | |
| "-T", | |
| type=str, | |
| help="Filter dispatches by their type (e.g., 'load', 'generation').", | |
| ) |
| <!-- Here goes a general summary of what this release is about --> | ||
|
|
||
| ## Upgrading | ||
|
|
||
| * You now must always provide the URL to the dispatch client. | ||
| <!-- Here goes notes on how to upgrade from previous versions, including deprecations and what they should be replaced with --> |
There was a problem hiding this comment.
Placeholder comments remain in the Summary and Upgrading sections; fill these with actual release highlights and migration notes before publishing.
llucax
left a comment
There was a problem hiding this comment.
LGTM, not sure if you have any tests for the CLI to add some tests for the additions...
Signed-off-by: Mathias L. Baumann <mathias.baumann@frequenz.com>