-
Notifications
You must be signed in to change notification settings - Fork 155
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add option to round up CPU quota #79
Conversation
35683e6
to
f5ff8d2
Compare
I just noticed there's a similar PR. This one's also related to the comments in #68 |
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.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #79 +/- ##
==========================================
+ Coverage 93.19% 93.96% +0.77%
==========================================
Files 9 10 +1
Lines 338 348 +10
==========================================
+ Hits 315 327 +12
+ Misses 19 16 -3
- Partials 4 5 +1 ☔ View full report in Codecov by Sentry. |
16897d2
to
57c88e3
Compare
Added a few tests for the new lines in runtime. I don't have linux so I couldn't run them on my local. I'll have to wait for the checks here 👀 |
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 finally set up a linux machine to run those tests! I also changed the implementation to take a round function in the option instead of setting the operation with a const value. I felt it was simpler to use and maintain. |
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.
looks good to me.
Does it make sense to test the round == nil case within internal/runtime since that's newly added?
Signed-off-by: Walther Lee <[email protected]>
@r-hang Sure! Added a test to check that the value is rounded down then the round arg is nil |
Add a
RoundQuotaFunc
option to control how the CPU quota should be rounded. The default is still rounding down (floor).This is related to issue #78