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

Add database health check #126

Merged
merged 4 commits into from
Mar 21, 2024

Conversation

Regenhardt
Copy link

This will return a json like this from the /health endpoint:

{
  "Status": "Healthy",
  "Sqlite": "Healthy"
}

If ListConfiguredServices is turned off, anything but the general Status is omitted.
Alternatively, we could not omit the specific checks entirely but just change the specific service name (e.g. Sqlite) to what the service is used for (e.g. Database).

The db health checks are identified using a tag that simply holds the service. That way any registered health check can be compared to the configuration so only active services are checked.
Alternatively (1), I could add the BaGetter tag to the health checks and use that as additional filter. That way, someone adding BaGetter to their own application could also have their own health checks named/tagged whatever without risk of collisions.
Alternatively (2), I could use the health check's name instead of a tag to identify it. Currently, the name is set to the service type set in the options.

Addresses #108

@seriouz
Copy link

seriouz commented Mar 21, 2024

A note in the docs on what you have changed would be nice: https://www.bagetter.com/docs/configuration#health-endpoint

Copy link

@seriouz seriouz left a comment

Choose a reason for hiding this comment

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

A small suggestion: What about naming the StatusProperty config variable StatusPropertyName? I think this is slightly clearer.

@Regenhardt Regenhardt force-pushed the feature/expanded-health-checks branch from e8cbb16 to d2045c3 Compare March 21, 2024 22:41
@Regenhardt Regenhardt merged commit 872afce into bagetter:main Mar 21, 2024
2 checks passed
@Regenhardt Regenhardt deleted the feature/expanded-health-checks branch March 21, 2024 23:06
@Regenhardt
Copy link
Author

Not sure how this ends up in the changelog. I don't usually work with squash commits.

@seriouz
Copy link

seriouz commented Mar 21, 2024

I am not sure either if the format is 100% correct. Lets do a smaller release in the near feature to see how this goes.

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.

2 participants