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: Enable health checks and add health check around the configuration #293

Merged
merged 4 commits into from
Oct 4, 2023

Conversation

killij
Copy link
Contributor

@killij killij commented Oct 4, 2023

Opinions on the static nature of the health checks welcome. We could scan the whole app config object and check all strings are populated but (i) some string are not used in prod - they could be in an ignore list & (ii) it could lead to unexpected prod deployment failure if new config was introduced and not configured correctly in the health check.

Have left as a static check for the moment but again new config needs to be manually added to it, which can also be overlooked. Opinions welcome.

@killij killij requested review from rgparkins and nnagepat October 4, 2023 09:56
Copy link
Contributor

@nnagepat nnagepat left a comment

Choose a reason for hiding this comment

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

Approved

@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 4, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

99.8% 99.8% Coverage
0.8% 0.8% Duplication

@killij killij merged commit 337c23c into main Oct 4, 2023
@rgparkins
Copy link
Contributor

🎉 This PR is included in version 1.22.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@killij killij deleted the feat/add-healthcheck branch November 7, 2023 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants