Add EventRecorder abstraction. #653
Conversation
Signed-off-by: Luca Palmieri <lpalmieri@truelayer.com>
LukeMathWalker
force-pushed
the
event-recorder-api
branch
from
1fb72d2
to
02788d0
Compare
Signed-off-by: Luca Palmieri <lpalmieri@truelayer.com>
Signed-off-by: Luca Palmieri <lpalmieri@truelayer.com>
Signed-off-by: Luca Palmieri <lpalmieri@truelayer.com>
Rename `instance_name` to `controller_pod_name`, including related structs. Signed-off-by: Luca Palmieri <lpalmieri@truelayer.com>
Signed-off-by: Luca Palmieri <lpalmieri@truelayer.com>
…he Debug representation. Signed-off-by: Luca Palmieri <lpalmieri@truelayer.com>
Signed-off-by: Luca Palmieri <lpalmieri@truelayer.com>
Signed-off-by: Luca Palmieri <lpalmieri@truelayer.com>
Signed-off-by: Luca Palmieri <lpalmieri@truelayer.com>
Signed-off-by: Luca Palmieri <lpalmieri@truelayer.com>
Signed-off-by: Luca Palmieri <lpalmieri@truelayer.com>
Signed-off-by: Luca Palmieri <lpalmieri@truelayer.com>
| pub struct ControllerPodName(String); | ||
|
|
||
| impl TryFrom<&str> for ControllerPodName { | ||
| type Error = ControllerPodNameParsingError; | ||
|
|
||
| fn try_from(value: &str) -> Result<Self, Self::Error> { | ||
| Self::try_from(value.to_string()) | ||
| } | ||
| } |
There was a problem hiding this comment.
I am slightly unsure of whether we should provide this type of convenience validation here. On one hand it is nice and can catch things early, but on the other hand we have no way to know to enforce that the constants we put inside our code matches the kubernetes source (and i would rather rely on the api-server rejecting this, than reject things that are valid in the future).
In this particular case as well; if people get the pod name from the downward api (as in your documentation), it would be impossible to get an illegal name here, so I'm leaning towards just having an unchecked String here.
There was a problem hiding this comment.
That said, it's nice for users to know as they are programming this. The event reason is very helpful for the user to know.
There was a problem hiding this comment.
This might be a question for maintainers for later on how we can do better client-side validation (without it getting out of date). For now, happy to leave this in. It's a nice convenience.
There was a problem hiding this comment.
Ideally this validation should live inside k8s_openapi, so that it can be associated to a specific version of the API server and can be changed to match when new versions are released.
Happy to remove if you prefer to delegate to k8s API server.
There was a problem hiding this comment.
(Or Kubernetes packages up its validation stuff into a target that can be bound against multiple programming languages, ensuring consistent and up-to-date validation across the entire ecosystem for each version. Can a man dream?)
There was a problem hiding this comment.
The sources that's used to generate the structs unfortunately does not have the information we need to do any type of newtype magic inside k8s-openapi or k8s-pb. So I think it's going to be a best-effort setup either way.
In this case, we are presenting an api on top of core::v1::Event though, so it's probably better to leave the validation herein.
There was a problem hiding this comment.
Then this is ready to go 😁
There was a problem hiding this comment.
Thank you very much! Merged it in. Started a discussion internally around the future of client-side validation in #654
Will otherwise release this in the next version (after the super-crate issue gets resolved I am thinking).
Signed-off-by: Luca Palmieri <lpalmieri@truelayer.com>
Signed-off-by: Luca Palmieri <lpalmieri@truelayer.com>
Signed-off-by: Luca Palmieri <lpalmieri@truelayer.com>
|
cargo deny failures are #650 - don't worry about it for now. |
Signed-off-by: Luca Palmieri <lpalmieri@truelayer.com>
A bunch of Clippy errors slipped in with kube-rs#653, which weren't picked up since Clippy was set up to only cover the main `kube` crate.
A bunch of Clippy errors slipped in with kube-rs#653, which weren't picked up since Clippy was set up to only cover the main `kube` crate. Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se>
A bunch of Clippy errors slipped in with kube-rs#653, which weren't picked up since Clippy was set up to only cover the main `kube` crate. Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se>
Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se>
Closes #249.