Add Header and Method support for HTTP checks #3031
Conversation
schmichael
changed the title
command/agent/consul/unit_test.go
Outdated
| Name: "name", | ||
| ServiceID: serviceID, | ||
| } | ||
| expected.Timeout = "0s" |
There was a problem hiding this comment.
Any reason these aren't inline with struct creation?
jobspec/parse.go
Outdated
| if headerI, ok := cm["header"]; ok { | ||
| headerRaw, ok := headerI.([]map[string]interface{}) | ||
| if !ok { | ||
| return fmt.Errorf("check -> header -> expected a []map[string]interface{} but found %T", headerI) |
There was a problem hiding this comment.
Should we show interface{} or []string since that is the only thing that is parsable.
| Interval: 10 * time.Second, | ||
| Timeout: 2 * time.Second, | ||
| InitialStatus: capi.HealthPassing, | ||
| Method: "POST", |
There was a problem hiding this comment.
Maybe a failing case? With numbers or something.
nomad/structs/diff.go
Outdated
| @@ -592,6 +592,23 @@ func serviceCheckDiff(old, new *ServiceCheck, contextual bool) *ObjectDiff { | |||
|
|
|||
| // Diff the primitive fields. | |||
| diff.Fields = fieldDiffs(oldPrimitiveFlat, newPrimitiveFlat, contextual) | |||
|
|
|||
| // Diff Header | |||
| headerDiff := &ObjectDiff{Type: DiffTypeNone, Name: "Header"} | |||
There was a problem hiding this comment.
Would you mind just popping it into a method?
schmichael
force-pushed
the
f-2924-consul-headers
branch
2 times, most recently
from
b76a221 to
77f4d96
Compare
schmichael
force-pushed
the
f-2924-consul-headers
branch
from
77f4d96 to
a6bf5b6
Compare
|
Travis failure is unrelated. |
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Fixes #2924
@dadgar Please check my work in diff.go. I haven't had to edit the diff logic before.
I modeled our header field after Consul's which is modeled after the Go stdlib which ... is kind of awkward for people not used to it for 2 reasons:
map[string][]stringinstead of[]string(like env vars)It's unfortunate that it's kind of an awkward API, but I default to consistency above trying to make a subjective UX tweak.