mysociety / mapit.mysociety.org

A Django project for running mySociety's main instance of MapIt for the UK
http://mapit.mysociety.org
Other
6 stars 2 forks source link

Support Sentinel for Redis connections #132

Closed sagepe closed 2 years ago

sagepe commented 2 years ago

We need to be able to support using Sentinel to mediate Redis connections.

sagepe commented 2 years ago

We use mockredis to, well, mock Redis in the tests. This is no longer maintained (and nor is the alternative they suggest using, fakeredis). Nor does it support mocking Sentinel. So if we want to add test coverage for this we'll need to take a wider look at this.

With REDIS_SENTINEL_HOSTS set, we do get the following error from the tests that suggest to me that it's not far off working:

redis.sentinel.MasterNotFoundError: No master found for 'data'

Set REDIS_SENTINEL_HOSTS to null in the exampel config file and tests pass using the usual connection, which at least suggests the logic to select Sentinel or direct connections works.

sagepe commented 2 years ago

An alternative to mocking would be to add redis and redis-sentinel services to the GitHub action, that would be simple enough to do. It would remove a dependency on the unsupported mockredis (although I did note that this has been forked by RedisLabs with recent commits) while making local testing more complex.