-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[usage] Add ReconcileUsage RPC #11676
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
Conversation
|
||
// CollectUsage collects usage for the specified time period, and stores the usage records in the database, returning the records. | ||
rpc CollectUsage(CollectUsageRequest) returns (CollectUsageResponse) {} |
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.
There is potential for confusion between this new RPC and the ListBilledUsage
RPC above.
AIUI, ListBilledUsage retrieves usage records (within a time range, for an attribution id) that have already been computed.
This new RPC actually triggers the usage collection that will populate usage records that are then available to ListBilledUsage
.
Maybe calling this one StartUsageReconciliation
or something similar, to emphasise its imperative nature would be better?
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.
In theory, calling either should be fine as the end result is going to be some (accurate) records in the DB. The main difference is one is a pure read, while the other is an Upsert in a way.
If the RPC is called Start..
, then I would expect it's an async job which it is not. Perhaps UpdateUsage
?
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.
Or ReconcileUsage, as that's probs most accurate
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.
Renamed in 7119163
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.
/hold for rename
/unhold |
Description
Adds RPC definition for Collecting usage
Related Issue(s)
How to test
Release Notes
Documentation
Werft options: