Skip to content

Conversation

@f0ssel
Copy link
Contributor

@f0ssel f0ssel commented Sep 21, 2022

Blocking coder/coder#4114

Copy link
Member

@kylecarbs kylecarbs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll leave more comments in a bit, just didn't have time for a full review!

Copy link
Member

@bpmct bpmct left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huge fan of the schema. Seems like its exactly what we're looking for

Comment on lines +390 to +401
"interval": {
Type: schema.TypeInt,
Description: "Duration in seconds to wait between healthcheck requests.",
ForceNew: true,
Required: true,
},
"threshold": {
Type: schema.TypeInt,
Description: "Number of consecutive heathcheck failures before returning an unhealthy status.",
ForceNew: true,
Required: true,
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor nit before merge, should we make these optional instead with reasonable defaults?

Also Kubernetes calls this period instead of interval. I'm impartial, just wanted to toss the idea up.

@f0ssel f0ssel merged commit 4d7165a into main Sep 22, 2022
@f0ssel f0ssel deleted the f0ssel/app-healthcheck branch September 22, 2022 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants