[PR] Allow to query config reports#1407
Open
knoppi wants to merge 1 commit intoSatelliteQE:masterfrom
Open
Conversation
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Renaming the
hostfield tohost_nameonReportis a breaking change for existing callers; consider either supporting both keys or adding a backwards-compatible alias/mapping to avoid runtime failures in existing code. - Changing the
_meta['api_path']fromapi/v2/reportstoapi/v2/config_reportsmay break any usage that relied on the previous endpoint; if both endpoints must remain usable, consider parameterizing or versioning the path instead of hard-switching it.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Renaming the `host` field to `host_name` on `Report` is a breaking change for existing callers; consider either supporting both keys or adding a backwards-compatible alias/mapping to avoid runtime failures in existing code.
- Changing the `_meta['api_path']` from `api/v2/reports` to `api/v2/config_reports` may break any usage that relied on the previous endpoint; if both endpoints must remain usable, consider parameterizing or versioning the path instead of hard-switching it.
## Individual Comments
### Comment 1
<location path="nailgun/entities.py" line_range="7182-7186" />
<code_context>
def __init__(self, server_config=None, **kwargs):
self._fields = {
- 'host': entity_fields.StringField(required=True),
+ 'host_name': entity_fields.StringField(required=True),
'logs': entity_fields.ListField(),
'reported_at': entity_fields.DateTimeField(required=True),
}
- self._meta = {'api_path': 'api/v2/reports'}
+ self._meta = {'api_path': 'api/v2/config_reports'}
super().__init__(server_config=server_config, **kwargs)
</code_context>
<issue_to_address>
**issue (bug_risk):** Renaming `host` to `host_name` and changing the API path may break existing consumers or deserialization.
Since this class now targets `api/v2/config_reports` and uses `host_name`, verify that:
- The API response actually uses `host_name` (not `host`), and
- No existing code or callers still depend on `Report.host` or the old `api/v2/reports` shape.
If there is existing usage, consider keeping `host` and adding `host_name` as an alias/derived field, or introducing a separate entity for config reports so the original `Report` interface remains unchanged.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
|
Can one of the admins verify this patch? |
55e7214 to
48cd98e
Compare
Contributor
Author
|
I changed the scope of this PR to adding a new class instead of modifying the class for legacy host reports. |
dosas
approved these changes
Mar 5, 2026
- New class for reading API endpoint config_reports - Entities are deletable, readable and searchable Note: The JSON response contains host_name unlike host in the response of the legacy reports API.
48cd98e to
0621e8a
Compare
Member
|
Waiting for second review. Is this something @SatelliteQE/team-endeavour wants to take a look at? |
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.
Description of changes
Enable querying the API endpoint for config reports.