Skip to content
This repository has been archived by the owner on Sep 9, 2024. It is now read-only.

slightly improved http error status codes and db health check #154

Merged
merged 5 commits into from
Jul 27, 2023

Conversation

chunningham
Copy link
Member

@chunningham chunningham commented Jul 26, 2023

Description

DB connection failing to be acquired from the connection pool currently results in a 401 unauthorised status code, when it should probably be a 500 or 503 status. This PR sets it to give a 503 status in this case. not necessarily a bug fix but will be more informative in future. Additionally, this pr sets the max number of connections to 100 (previously defaulted to 10) and adds a db status check to the /healthz endpoint.

Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Diligence Checklist

(Please delete options that are not relevant)

  • This change requires a documentation update
  • I have included unit tests
  • I have updated and/or included new integration tests
  • I have updated and/or included new end-to-end tests
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

@chunningham chunningham changed the title slightly improved http error status codes slightly improved http error status codes and db health check Jul 26, 2023
@chunningham
Copy link
Member Author

@sbihel the default max connections was 10, I've set it to 100, perhaps that's too high but I'm not sure how to estimate an appropriate value for a given deployment. I could also add it as a config option

@sbihel
Copy link
Member

sbihel commented Jul 27, 2023

Yeah potentially making it configurable would be nice. I think 100 is fine for now

@chunningham chunningham merged commit 4b91119 into main Jul 27, 2023
12 checks passed
@chunningham chunningham deleted the fix/better-error branch July 27, 2023 11:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants