-
Notifications
You must be signed in to change notification settings - Fork 328
Enable custom health checks for ECS plugin #4473
Conversation
270ee41
to
fa32f17
Compare
if c.HealthCheck != nil { | ||
if c.HealthCheck.Timeout >= c.HealthCheck.Interval { | ||
return status.Errorf(codes.FailedPrecondition, "the health check "+ | ||
"timeout must be less than the interval") |
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.
We should include the interval and timeout in the error message!
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.
Added the doc fields to Documentation
, but make gen/website-mdx
didn't generate the updated .mdx files 🤔 still looking into it.
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.
If I remember right, if it's a new markdown file, you have to manually @include
it in like: https://github.com/hashicorp/waypoint/blob/main/website/content/plugins/aws-ec2.mdx. Not sure if that's the issue
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.
The ECS platform plugin docs indeed are included. Even more weirdly, this is passing the CircleCI website-mdx check... maybe it's because all of the fields in the new struct are optional?
builtin/aws/ecs/platform.go
Outdated
|
||
// Matcher is the HTTP codes to use when checking for a successful response from a target. | ||
// The range is 200 to 599. The default is 200-399. | ||
Matcher *elbv2.Matcher `hcl:"matcher,optional"` |
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.
We should document this config struct in the Documentation()
func below!
c26c284
to
41d0136
Compare
builtin/aws/ecs/platform.go
Outdated
"Health check settings for the app.", | ||
docs.SubFields(func(doc *docs.SubFieldDoc) { | ||
doc.SetField("protocol", | ||
"The protocol for the health check to use.") |
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.
Not sure if our docs have a special field for this, but it would be nice to say what the valid protocols are!
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.
minor comments
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.
👍 on the changes after my feedback
3745e55
to
695536f
Compare
eb2e2b2
to
77033fb
Compare
Closes #4455. Co-authored with @cicoyle.