-
Couldn't load subscription status.
- Fork 13
Renaming to allow for api-reporting update #205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Renaming to allow for api-reporting update #205
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the client’s reporting layer to match an upstream API change by renaming timestamp and filter parameters.
- Renamed
sampled_attosample_timein metrics and states iteration and aggregation - Updated
PBTimeFiltercalls to usestart_time/end_timeinstead ofstart/end - Propagated field-name changes in both the iterator and aggregation client
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/frequenz/client/reporting/_types.py | Renamed sampled_at accesses to sample_time in iteration and in AggregatedMetric.sample |
| src/frequenz/client/reporting/_client.py | Updated two PBTimeFilter constructions to use start_time/end_time parameters |
Comments suppressed due to low confidence (1)
src/frequenz/client/reporting/_types.py:95
- Add or update unit tests to verify that the new
sample_timefield is correctly converted for all metric samples, ensuring backward compatibility of timestamp handling.
ts = sample.sample_time.ToDatetime().replace(tzinfo=timezone.utc)
5429a06 to
4ebf5f3
Compare
4ebf5f3 to
3b4a6f0
Compare
3b4a6f0 to
785d79e
Compare
|
Sorry, I need a re-review on this. Only picked up the protobuf dependency when it failed the merge queue... |
Signed-off-by: Flora <[email protected]>
785d79e to
0668841
Compare
No description provided.