feat: Add ProbeBuilder #1078
feat: Add ProbeBuilder #1078
Conversation
| #[derive(Debug)] | ||
| pub struct ProbeBuilder<Action, Period> { |
There was a problem hiding this comment.
I do think we should have docs on pub items.
It doesn't have to be super comprehensive, and examples can be added to the module level docs.
| #[derive(Debug)] | |
| pub struct ProbeBuilder<Action, Period> { | |
| /// Valid Kubernetes probe builder | |
| /// | |
| /// The upstream struct [NameOfStruct] does not prevent invalid probe configurations | |
| /// which leads to surprises at runtime which can be deeply hidden. | |
| #[derive(Debug)] | |
| pub struct ProbeBuilder<Action, Period> { |
| #[derive(Debug, Snafu)] | ||
| pub enum Error { | ||
| #[snafu(display( | ||
| "The probe's {field:?} duration of {duration} is too long, as it's seconds doesn't fit into an i32" |
There was a problem hiding this comment.
How does that appear? Does it look like human time?
Eg:
duration of 4,734e+8 is too longduration of 15 is too long, orduration of 15 years is too long
| } | ||
| } | ||
|
|
||
| pub enum ProbeAction { |
There was a problem hiding this comment.
To save documenting each variant, we can refer to upstream docs.
| pub enum ProbeAction { | |
| /// Available probes | |
| /// | |
| /// Only one probe can be configured at a time. For more details about each | |
| /// type, see [container-probes] documentation. | |
| /// | |
| /// [container-probes]: https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle/#container-probes | |
| pub enum ProbeAction { |
| /// There is a convenience helper: [`Self::with_exec_action_helper`]. | ||
| pub fn with_exec_action(self, exec_action: ExecAction) -> ProbeBuilder<ProbeAction, ()> { |
There was a problem hiding this comment.
| /// There is a convenience helper: [`Self::with_exec_action_helper`]. | |
| pub fn with_exec_action(self, exec_action: ExecAction) -> ProbeBuilder<ProbeAction, ()> { | |
| /// Set an exec probe with the given configuration. | |
| /// | |
| /// For simple cases, there is a a convenience helper: [`Self::with_exec_action_helper`]. | |
| pub fn with_exec_action(self, exec_action: ExecAction) -> ProbeBuilder<ProbeAction, ()> { |
There was a problem hiding this comment.
I'll stop adding documenting comments, but this should apply to other pub methods too.
| let probe = ProbeBuilder::default() | ||
| .with_exec_action_helper(["sleep", "1"]) | ||
| .with_period(Duration::from_secs(5)) |
There was a problem hiding this comment.
Part of me feels like the word "helper" should be replaced with what it does.
But actually, I have a different recommendation overall...
Since there are a small finite set of probe types that can be created, we should drop the default impl... and make the developer choose one...
let probe = ProbeBuilder::exec_with_command(["sleep", "1"])
.with_period(Duration::from_secs(5))and for the non "helper" case, it could just be:
let probe = ProbeBuilder::exec(the_exec_stuff)
.with_period(Duration::from_secs(5))There was a problem hiding this comment.
To expand on things... I don't believe there is a good "default" probe, and therefore there shouldn't be a default method.
Description
Adds a builder for a
Probesimilar to theResourceRequirementsBuilderNeeded for stackabletech/superset-operator#654
Definition of Done Checklist
Author
Reviewer
Acceptance