-
Notifications
You must be signed in to change notification settings - Fork 355
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
Improvements to cgroup support #513
Conversation
9864db0
to
8477d48
Compare
Codecov Report
@@ Coverage Diff @@
## main #513 +/- ##
==========================================
+ Coverage 60.54% 60.86% +0.32%
==========================================
Files 86 87 +1
Lines 12604 12672 +68
==========================================
+ Hits 7631 7713 +82
+ Misses 4973 4959 -14 |
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. Minor nit picks, and if you disagree feel free to resolve.
crates/libcgroups/src/common.rs
Outdated
|
||
#[cfg(not(feature = "v1"))] | ||
fn create_v1_cgroup_manager(_cgroup_path: PathBuf) -> Result<Box<dyn CgroupManager>> { | ||
bail!("cgroup v1 feature is not activated"); |
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.
Maybe for these features we should add some context around the fact that the build excluded this feature? I know we didn't do this in the past, but maybe it's worth doing moving forward.
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 really appreciate this being broken out and put behind feature flags. I think this is much cleaner.
8477d48
to
af4d403
Compare
@tsturzl PTAL |
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.
This looks great
This contains multiple smaller improvements that did not really warrant their own PR.