From 7e26cb5baf36fc77e39491d21550a4975843a5ee Mon Sep 17 00:00:00 2001 From: David McDonald Date: Thu, 9 Dec 2021 14:16:36 +0000 Subject: [PATCH] Quick fix to redis DEFAULT_TTL type bug In https://github.com/alphagov/notifications-admin/pull/4077/commits/a9617d4df6021c3080bf888c9f0e5cca4b72044d we upgraded the version of utils to 49.1 which brought in a renamed `TTL` as `DEFAULT_TTL`. However, not only did it change the name, it also changed its type from an `int` to a `float`: https://github.com/alphagov/notifications-utils/pull/923/files We thought that would be OK as in the utils, we moved the conversion to an integer to happen in the `set` method but it turns out that caused an issue in the admin app where setting the `has_jobs...` redis keys will error: ``` Redis error performing set on has_jobs-4bd11cb2-cc17-44e1-b241-8547990db245 ... ... redis.exceptions.ResponseError: value is not an integer or out of range ``` It looks like this is because we are passing a float instead of an int to `ex` See a similar post describing the importance of ints rather than floats for other parameters: https://developpaper.com/question/redis-err-value-is-not-an-integer-or-out-of-range/ An interesting note is our test `test_client_creates_job_data_correctly` didn't catch this because `float(604800) == int(604800`. I've gone for the quickest solution which is to wrap `DEFAULT_TTL` in an int. The reason I've done this now is that to do the long term and more durable fix is to add this fix to utils, however there are several breaking changes infront of it that would take me a while to bring in to the admin app first. I've checked the admin and API apps and this is the only place we are directly using `DEFAULT_TTL`. --- app/notify_client/job_api_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/notify_client/job_api_client.py b/app/notify_client/job_api_client.py index 115ece59e..ee9dc0022 100644 --- a/app/notify_client/job_api_client.py +++ b/app/notify_client/job_api_client.py @@ -103,7 +103,7 @@ class JobApiClient(NotifyAdminAPIClient): redis_client.set( 'has_jobs-{}'.format(service_id), b'true', - ex=cache.DEFAULT_TTL, + ex=int(cache.DEFAULT_TTL), ) return job