Skip to content

Conversation

@mnorbury
Copy link
Contributor

  • Fixing API to match the example usage
  • Add a quota monitoring function

@github-actions github-actions bot added the feat label Oct 25, 2023
@github-actions
Copy link

github-actions bot commented Oct 25, 2023

⏱️ Benchmark results

Comparing with 5553f85

  • Glob-8 ns/op: 90.48 ⬇️ 0.28% decrease vs. 5553f85

}

func (qc quotaChecker) startQuotaMonitor(ctx context.Context) (context.Context, func()) {
newCtx, cancel := context.WithCancel(ctx)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typically a cancelable context is created by the caller, but in this case I didn't want to make the caller (plugin developer) have to write this code.

case <-newCtx.Done():
return
case <-ticker.C:
hasQuota, err := qc.qm.HasQuota(newCtx)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to add retries to the HasQuota call.

case <-ticker.C:
hasQuota, err := qc.qm.HasQuota(newCtx)
if err != nil {
continue
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What should we do with an error determining the quota - since returning an error here would cancel the sync.

@mnorbury mnorbury force-pushed the fix/make-api-compatible-with-example-plugin branch from c089a68 to 84ac02c Compare October 26, 2023 09:43
@mnorbury mnorbury force-pushed the fix/make-api-compatible-with-example-plugin branch 2 times, most recently from 9e3a155 to 4b86abc Compare October 26, 2023 15:38
@mnorbury mnorbury force-pushed the fix/make-api-compatible-with-example-plugin branch from 4b86abc to 68aade0 Compare October 27, 2023 10:01
@mnorbury
Copy link
Contributor Author

Split into #1333 and #1334

@mnorbury mnorbury closed this Oct 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants