Validate flow as told in the flow spec comments.#398
Conversation
knative-prow-robot
added
the
size/L
|
/assign @vaikas-google We have moved to using the knative/pkg/webhooks to use the .Validate() method on our resources. This cleaned up a lot of code, check out the linked PR too :D |
|
/test pull-knative-eventing-go-coverage |
|
/test pull-knative-eventing-go-coverage |
|
The following is the coverage report on pkg/.
|
| Action: FlowAction{ | ||
| TargetURI: stringPtr("http://example.com/action"), | ||
| }, | ||
| Trigger: EventTrigger{ |
There was a problem hiding this comment.
This gets recreated a few times. Maybe pull it into a variable?
validEventTrigger := ...
| }, | ||
| want: &apis.FieldError{ | ||
| Message: `invalid value "http//example.com/action"`, | ||
| Paths: []string{"action.targetURI"}, |
There was a problem hiding this comment.
I am still catching up with the changes for validation code while I was gone. I like what's being done with them, I'm just little confused on what the eventual returned to the user and how the paths get stitched together. But that's not really part of this PR, just need to go read some more code :)
There was a problem hiding this comment.
The idea is that the field context is prefixed as you come back up the call stack, so you can traverse a object graph and call validate on sub-objects. If those objects throw errors, you add the context only you know about in the context the object is used. IE:
err = obj.AnotherObj.Validate()
if err != nil {
return err.ViaField("AnotherObj")
}
AnotherObj's type was able to have a validate function that is only aware of the objects requirements. Only obj knew the context in which the user knows how they used AnotherObj.
Check out knative/pkg/apis
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: n3wscott, vaikas-google The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
knative-prow-robot
added
the
approved
|
/test pull-knative-eventing-build-tests |
* Patch for optional chmod for codegen scripts * Apply codegen patch * Also apply patch after running udpate-codegen.sh
5 participants
Follow-up to #352
Proposed Changes
Release Note