add parse capabilities for duration on event command#3214
add parse capabilities for duration on event command#3214mdaffad wants to merge 9 commits intoyouki-dev:mainfrom
Conversation
mdaffad
force-pushed
the
enhancing-runc-compatibility-for-interval-on-event
branch
from
9ec332e to
b10c7aa
Compare
|
Hey, can you rebase and fix the clippy issues? Thanks :) |
|
Updated |
|
@mdaffad Please sign following the instructions in the link |
crates/liboci-cli/src/events.rs
Outdated
|
|
||
| let num = num.parse::<u64>().map_err(|_| "Invalid number")?; | ||
|
|
||
| let dur = match unit.as_str() { |
mdaffad
force-pushed
the
enhancing-runc-compatibility-for-interval-on-event
branch
2 times, most recently
from
b165eae to
2214ff1
Compare
|
@mdaffad May I ask you to address DCO? |
crates/liboci-cli/Cargo.toml
Outdated
| keywords = ["youki", "container", "oci"] | ||
|
|
||
| [dependencies] | ||
| parse_duration = "2.1.1" |
There was a problem hiding this comment.
Can't we implement it ourselves?
Signed-off-by: Muhammad Daffa Dinaya <muhammaddaffadinaya@gmail.com>
Signed-off-by: Muhammad Daffa Dinaya <muhammaddaffadinaya@gmail.com>
Signed-off-by: Muhammad Daffa Dinaya <muhammaddaffadinaya@gmail.com>
Signed-off-by: Muhammad Daffa Dinaya <muhammaddaffadinaya@gmail.com>
Signed-off-by: Muhammad Daffa Dinaya <muhammaddaffadinaya@gmail.com>
Signed-off-by: Muhammad Daffa Dinaya <muhammaddaffadinaya@gmail.com>
Signed-off-by: Muhammad Daffa Dinaya <muhammaddaffadinaya@gmail.com>
Signed-off-by: Muhammad Daffa Dinaya <muhammaddaffadinaya@gmail.com>
mdaffad
force-pushed
the
enhancing-runc-compatibility-for-interval-on-event
branch
from
54974f2 to
5d7bffc
Compare
Signed-off-by: Muhammad Daffa Dinaya <muhammaddaffadinaya@gmail.com>
| let s = s.trim(); | ||
| if let Ok(secs) = s.parse::<u64>() { | ||
| return Ok(Duration::from_secs(secs)); | ||
| } |
There was a problem hiding this comment.
It looks like runc is not supported.
$ sudo runc events --interval 1 tutorial_container
Incorrect Usage: invalid value "1" for flag -interval: parse error
NAME:
runc events - display container events such as OOM notifications, cpu, memory, and IO usage statistics
USAGE:
runc events [command options] <container-id>
Where "<container-id>" is the name for the instance of the container.
DESCRIPTION:
The events command displays information about the container. By default the
information is displayed once every 5 seconds.
OPTIONS:
--interval value set the stats collection interval (default: 5s)
--stats display the container's stats then exit
ERRO[0000] invalid value "1" for flag -interval: parse error
There was a problem hiding this comment.
I see. I think we should drop the digit only then.
| Ok(Duration::from_secs(total_seconds)) | ||
| } | ||
|
|
||
| fn parse_interval(s: &str) -> Result<Duration, String> { |
There was a problem hiding this comment.
Please update the return value to use an Error type instead of a String, to stay consistent with the implementation in exec.rs.
| fn parse_interval(s: &str) -> Result<Duration, String> { | |
| fn parse_interval(s: &str) -> Result<Duration, Box<dyn Error + Send + Sync + 'static>> { |
ref: https://github.com/youki-dev/youki/blob/main/crates/liboci-cli/src/exec.rs#L67-L92
| fn parse_duration_string(s: &str) -> Result<Duration, String> { | ||
| let s = s.trim().to_lowercase(); | ||
| let mut total_seconds = 0u64; | ||
| let mut current_number = String::new(); |
There was a problem hiding this comment.
Using current_number.push(ch) in a loop allocates a String and can cause repeated reallocations. Prefer a slice-based approach: work on &[u8] (or &str), use split_at and a remainder slice so you don't need a digit buffer. That avoids extra allocations and is more efficient.
|
|
||
| use clap::Parser; | ||
|
|
||
| fn parse_duration_string(s: &str) -> Result<Duration, String> { |
There was a problem hiding this comment.
Could you also add some unit tests for this logic?
4 participants
Description
Type of Change
Testing
Related Issues
Close #3206
Additional Context