-
-
Notifications
You must be signed in to change notification settings - Fork 325
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
Finalizer helper #475
Finalizer helper #475
Conversation
Imported from https://gitlab.com/teozkr/kube-finalizer, with some docs cleanup.
kube-runtime/src/finalizer.rs
Outdated
) -> Result<ReconcilerAction, Error<ApplyFut::Error, CleanupFut::Error>> | ||
where | ||
K: Metadata<Ty = ObjectMeta> + Clone + DeserializeOwned + Serialize + Debug, | ||
ApplyFut: TryFuture<Ok = ReconcilerAction>, | ||
ApplyFut::Error: StdError + 'static, | ||
CleanupFut: TryFuture<Ok = ()>, | ||
CleanupFut::Error: StdError + 'static, |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
(Copied and adapted from kube-finalizer)
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 think this is great. It looks like this resolves all my previous concerns about it being hard to read, and it also looks like you've found a really nice way to nest the error types with one error type in your finalizer that nests the users error type that must be the same in apply
and cleanup
.
I don't think this needs any real changes before merging. Have only left minor readability nits on the example.
released in 0.58 :-) |
Imported from https://gitlab.com/teozkr/kube-finalizer, with some docs cleanup.
Fixes #291
Not super happy with the API, but think it would still be useful to get some external feedback..