Improve validation message on build spec update #9164
Conversation
|
Should help to diagnose openshift/jenkins-sync-plugin#70 |
csrwng
force-pushed
the
build_put_validation
branch
3 times, most recently
from
14db1d9 to
f544564
Compare
| if err != nil { | ||
| return "", fmt.Errorf("error converting newer to versioned: %v", err) | ||
| } | ||
| patch, err := strategicpatch.CreateTwoWayMergePatch(olderJSON, newerJSON, versioned) |
There was a problem hiding this comment.
can versioned just be an empty build? I'm pretty sure it just uses it navigate metadata for strategic merge patches.
|
I think we may want to refactor it more generally, but this is a good start. |
csrwng
force-pushed
the
build_put_validation
branch
from
f544564 to
c0a3cce
Compare
|
comment addressed |
| }, | ||
| }, | ||
| }, | ||
| expected: `{"Spec":{"Source":{"ContextDir":"context-dir"}}}`, |
There was a problem hiding this comment.
you shouldn't be getting caps. Convert to external first.
There was a problem hiding this comment.
which external? I didn't want to assume 'v1' is that ok?
There was a problem hiding this comment.
which external? I didn't want to assume 'v1' is that ok?
Yeah, v1 is closer than anything else right now.
csrwng
force-pushed
the
build_put_validation
branch
from
c0a3cce to
8592893
Compare
|
using v1 now |
|
btw, I'd considered using v1 before, but wasn't sure that it was proper to reference a specific version directly. |
It's not really latest. Validation currently chooses an API version that its reported tags (the paths) are going to match. You should probably match whatever that is. |
|
lgtm [merge] |
|
[Test]ing while waiting on the merge queue |
|
Evaluated for origin test up to 8592893 |
|
continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/4439/) |
|
[merge] |
2 similar comments
|
[merge] |
|
[merge] |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/4543/) (Image: devenv-rhel7_4325) |
|
[merge] |
|
Evaluated for origin merge up to 8592893 |
Includes a diff of the build specs (old/new) in the error message details.