Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Convert waiting and starting status to 202 #2161

Merged
merged 3 commits into from
Dec 12, 2024

Conversation

msfstef
Copy link
Contributor

@msfstef msfstef commented Dec 12, 2024

Before we were returning 503s for anything other than active status - for some health check services (like AWS Fargate) you can only rely on the status code, and technically Electric in waiting and active status is ready to accept requests, just not service them.

I've changed it so that:

  • waiting and starting return 202, requests are accepted and held but failed after a timeout if not activated
  • active returns 200
  • stopping returns 503, we haven't implemented it yet anyway

Not sure if we should consider this a breaking change and how we handle releases at the moment @KyleAMathews .

Copy link
Contributor

@KyleAMathews KyleAMathews left a comment

Choose a reason for hiding this comment

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

Seems more like a bug fix than a breaking change so :shipit:

Copy link
Contributor

@robacourt robacourt left a comment

Choose a reason for hiding this comment

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

👍

@msfstef
Copy link
Contributor Author

msfstef commented Dec 12, 2024

Added a HEALTHCHECK to the dockerfile as well, so that we leverage Docker's "healthy" status too

@msfstef msfstef merged commit 7caccbf into main Dec 12, 2024
32 checks passed
@msfstef msfstef deleted the msfstef/health-check-status-codes branch December 12, 2024 16:35
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.

3 participants