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

[SQL Health Check] Handle HTTP exceptions #902

Merged
merged 8 commits into from
Feb 15, 2024

Conversation

fhibf
Copy link
Contributor

@fhibf fhibf commented Feb 13, 2024

SQL Health Check - Handle HTTP exception

Related issues

  • FHIR Health Checks can fail due AAD misconfiguration. And those failures can happen when one of our internal components try to acquire the connection string to SQL.

Testing

Added new unit test to ensure errors in SQL Health Checks are handled properly.

Copy link
Member

@jnlycklama jnlycklama left a comment

Choose a reason for hiding this comment

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

Is there also some way to check if the access is controlled by the customer? Maybe we can pass in a config or something that will have a flag if the SQL connection is customer controlled?

I'm worried about this filtering out health check failures that are caused by actual bugs. For example, in Dicom SQL is completely internal so there's no way a customer could mess anything up. If we get a Forbidden or Unauthorized exception, we would want to throw an exception here because that means something to do with our token is messed up.

@fhibf fhibf enabled auto-merge (squash) February 15, 2024 00:28
@fhibf fhibf changed the title [SQL Health Check] Handle HTTP exception [SQL Health Check] Handle HTTP exceptions Feb 15, 2024
@fhibf fhibf added the bug Something isn't working label Feb 15, 2024
@fhibf fhibf merged commit 66260c2 into main Feb 15, 2024
7 checks passed
@fhibf fhibf deleted the personal/fernfe/sqlHealthCheckHandleHttpException branch February 15, 2024 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants