This repository has been archived by the owner on Sep 26, 2023. It is now read-only.
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.
feat: dynamic flow control p3: add FlowControllerEventStats #1332
feat: dynamic flow control p3: add FlowControllerEventStats #1332
Changes from 6 commits
557faca
76d8c39
2985152
5fe9b2d
fb0c59d
0ab107d
f0357ba
0e2f63b
317489c
e51ff12
74eae2b
5e3cb67
ed83ec1
522be6c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Subjective, but it looks like this class does not need to be
AutoValue
. It is way less trivial than most of theAutoValue
classes, which are usually just a set of setters/getters (not the case here). Please consider just storing thethrottledtimeInMs
,exception
andtimestampMs
values manually in this class.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.
agreed
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.
AutoValue doesnt actually hurt anything here. I generally prefer AutoValue here to communicate the fact that this is a simple value type