-
Notifications
You must be signed in to change notification settings - Fork 97
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
Better parse error handling #270
Comments
Re. |
Yeah, Plays def listReadsCollectingErrors[Sp: Reads, St: Reads]: Reads[(List[(CustomResource[JsValue, JsValue], JsError)], List[CustomResource[Sp, St])] = {
implicitly[Reads[List[CustomResource[JsValue, JsValue]]]].map { resources =>
val results = resources.map { jsResource =>
Json.toJson(jsResource).validate[CustomResource[Sp, St]] match {
case JsSuccess(resource) => Right(resource)
case err: JsError => Left((jsResource, err))
}
}
(jsResults.collect { case Left(err) => err }, jsResults.collect { case Right(resource) => resource })
}
} The reason that the it's a list of As for |
I am coming around to a preference for adding an enriched event type that can represent either errors or normal events. That would require new API watch methods that return sources of these new event types and more than likely deprecating the old watch methods, I'll need to flesh that out. |
Working with custom resources in Skuber is very difficult due to parse errors being fatal. Skuber assumes that parsing will always be successful, and doesn't offer any opportunity to handle parse errors gracefully. So, if you're not using validation in your CRDs (or if you don't have the validation quite right), then an end user updating a malformed resource will result in the entire operator, for both good and the bad resource, from ceasing to work - any events that are received for that resource will cause the watch source to fail, and any list operations for that type of resource will fail.
Here's what I would suggest should be modified to make writing robust operators much simpler:
WatchEvent
should have an additional property (or be wrapped in a higher level object) that can notify when a parse error is encountered. This will allow an operator to see the parse error, and update the status of the resource with an error, to allow the user to learn what they did wrong.ResourceList
should have an additional property for items that were failed to be parsed, and theFormat
for it should capture all parse failures, and instead of failing to parse the entire list, put the errors into this additional property. This will allow the operator, when it does its initial request for the current set of resources before starting the poll, to set an error in the status of those incorrectly formatted resources.With the following modifications (or equivalent), I think it will be much easier to write operators using skuber.
The text was updated successfully, but these errors were encountered: