client/batch: allow tokenCh of batch controller to be nil #8903
Conversation
Signed-off-by: JmPotato <[email protected]>
ti-chi-bot
bot
added
release-note-none
|
/cc @MyonKeminta |
|
@JmPotato: GitHub didn't allow me to request PR reviews from the following users: MyonKeminta. Note that only tikv members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
ti-chi-bot
bot
added
the
size/L
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8903 +/- ##
=======================================
Coverage 76.25% 76.25%
=======================================
Files 461 461
Lines 70395 70399 +4
=======================================
+ Hits 53677 53685 +8
+ Misses 13372 13365 -7
- Partials 3346 3349 +3
Flags with carried forward coverage won't be shown. Click here to find out more. |
Signed-off-by: JmPotato <[email protected]>
JmPotato
changed the title
|
@MyonKeminta: adding LGTM is restricted to approvers and reviewers in OWNERS files. In response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
ti-chi-bot
bot
added
needs-1-more-lgtm
| @@ -60,13 +60,18 @@ func NewController[T any](maxBatchSize int, finisher FinisherFunc[T], bestBatchO | |||
| // It returns nil error if everything goes well, otherwise a non-nil error which means we should stop the service. | |||
| // It's guaranteed that if this function failed after collecting some requests, then these requests will be cancelled | |||
| // when the function returns, so the caller don't need to clear them manually. | |||
| // `tokenCh` is an optional parameter: | |||
There was a problem hiding this comment.
Shall we use something like
FetchPendingRequests(ctx context.Context, requestCh <-chan T, maxBatchWaitInterval time.Duration, tokenCh ...chan struct{}) (errRet error) {
There was a problem hiding this comment.
It looks good to me, but I prefer keeping the current way to force the caller to decide whether tokenCh is needed by explicitly passing it in.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: MyonKeminta, nolouch, rleungx The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What problem does this PR solve?
Issue Number: ref #8690.
What is changed and how does it work?
Check List
Tests
Release note