- Notifications
You must be signed in to change notification settings - Fork921
chore: move Batcher and Tracker to workspacestats#13418
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
coderd/workspacestats/batcher.go Outdated
| type StatsBatcher interface { | ||
| Add(now time.Time, agentID uuid.UUID, templateID uuid.UUID, userID uuid.UUID, workspaceID uuid.UUID, st *agentproto.Stats) error | ||
| } |
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.
Why do we have this interface and the batcher?
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's a test implementation athttps://github.com/coder/coder/blob/f0ssel/move-batchers/coderd/agentapi/stats_test.go#L30. That being said, there's a duplicate definition of this interface athttps://github.com/coder/coder/blob/f0ssel/move-batchers/coderd/agentapi/stats.go#L19 that can be removed.
| batcher, closeBatcher, err :=workspacestats.NewBatcher(ctx, | ||
| workspacestats.BatcherWithLogger(options.Logger.Named("batchstats")), | ||
| workspacestats.BatcherWithStore(options.Database), |
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.
I'm not particularly married to the functional opts here if you want to use a more conventional opts struct instead or just straight-up function args.
Uh oh!
There was an error while loading.Please reload this page.
5b9a65e intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
This moves the
batchstats.Batcherandworkspaceusage.Trackerinto theworkspacestatspackage to keep all updates tolast_used_atinside the same package. There are no functional changes in this PR.