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

Fix /health/info Redis connection #411

Merged
merged 9 commits into from
Sep 7, 2023
Merged

Fix /health/info Redis connection #411

merged 9 commits into from
Sep 7, 2023

Conversation

Jozzey
Copy link
Contributor

@Jozzey Jozzey commented Sep 6, 2023

https://eaflood.atlassian.net/browse/WATER-4099

After adding the missing information to the /health/info page in PR #376 It was discovered that whilst it worked fine locally, when viewed on the dev server Redis was unable to connect. This was down the Redis connection on the AWS servers needing TLS to be enabled.

This PR will enable TLS by default for the Redis connection & add an environment variable to enable TLS to be disabled locally.

https://eaflood.atlassian.net/browse/WATER-4099

After adding the missing information to the `/health/info` page in PR #376 It was discovered that whilst it worked fine locally, when viewed on the `dev` server Redis was unable to connect. This was down the Redis connection on the AWS servers needing TLS to be enabled.

This PR will enable TLS by default for the Redis connection & add an environment variable to enable TLS to be disabled locally.

Whilst debugging this issue it was also found that if a connection to Redis could not be established the page would error rather than just display a connection failure message for Redis and continue to show the status of the other services. This behaviour was not present locally. After much hacking a fix has not be found for this issue. Given we have more urgent work currently this issue will be added to our issues backlog to investigate further at a later date.
@Jozzey Jozzey added the bug Something isn't working label Sep 6, 2023
@Jozzey Jozzey self-assigned this Sep 6, 2023
@Jozzey Jozzey marked this pull request as ready for review September 6, 2023 20:55
Cruikshanks and others added 2 commits September 7, 2023 09:52
Found this post <https://stackoverflow.com/a/61133867> which put me onto some additional properties we can pass in the config that make the client throw an error as expected when a command fails to complete rather than using the under-the-hood Redis error handling.

This means should we fail to connect an error will be thrown which we can trap and display in our health info page.
@Jozzey Jozzey merged commit a67631b into main Sep 7, 2023
@Jozzey Jozzey deleted the fix-health-info branch September 7, 2023 09:00
@Jozzey
Copy link
Contributor Author

Jozzey commented Sep 8, 2023

PR linked to this issue DEFRA/water-abstraction-team#54

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