Conversation
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3055 +/- ##
==========================================
+ Coverage 58.26% 58.36% +0.10%
==========================================
Files 2072 2082 +10
Lines 171414 171146 -268
==========================================
+ Hits 99882 99897 +15
+ Misses 62602 62313 -289
- Partials 8930 8936 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
| // The work pool shuts down when the context is done. | ||
| ctx context.Context, | ||
| // The name of the work pool. Used for metrics. | ||
| name string, |
There was a problem hiding this comment.
Seems currently not being used, are we gonna add metrics for that in the future
There was a problem hiding this comment.
Yes, the plan was to add metrics, but I didn't get to that as a part of this PR. I still want to add them eventually, though.
What is your preference? I'm ok with either deleting this until we need (not that much work to add later), or leaving it in as a placeholder.
There was a problem hiding this comment.
Sounds good, if we plan to add metrics in the future, let's keep the name there
sei-db/common/unit/data_units.go
Outdated
| ZB = EB * 1024 | ||
| // YB is the number of bytes in a yottabyte. | ||
| YB = ZB * 1024 |
There was a problem hiding this comment.
Any attempt to assign them to a standard integer variable (e.g., var x int64 = unit.ZB) will fail at compile time, this might be a bit confusing, do you think we would need to use that unit at all? Or shall we just remove them for now and add them when we really need it?
There was a problem hiding this comment.
Lol, very good point. I've removed the ZB and YB constants.
| } | ||
|
|
||
| func (p *adHocPool) Submit(_ context.Context, task func()) error { | ||
| go task() |
There was a problem hiding this comment.
- fixedPool and elasticPool both return an error for nil tasks but adHocPool launches
go task()directly. consider adding a nil check. - the passed in context cancellation is ignored
There was a problem hiding this comment.
ixedPool and elasticPool both return an error for nil tasks but adHocPool launches go task() directly. consider adding a nil check.
check added
the passed in context cancellation is ignored
This now checks to see if the context is done before invoking
Describe your changes and provide context
Implements several utilities needed for the FlatKV cache.
Testing performed to validate your change
Unit tests, tested on branch that has integrated cache