Skip to content

Commit

Permalink
Change how seeded data works (#187)
Browse files Browse the repository at this point in the history
This PR will allow us to change how seeded data works. Instead of having a different data structure in Redis for seeded data, we can just do a bulk insert of the seeded data into the same data structure we use normally.
  • Loading branch information
smcmurtry authored Apr 24, 2023
1 parent d7e6723 commit 80fd385
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 96 deletions.
74 changes: 23 additions & 51 deletions notifications_utils/clients/redis/bounce_rate.py
Original file line number Diff line number Diff line change
@@ -1,74 +1,46 @@
"""This module is used to calculate the bounce rate for a service. It uses Redis to store the total number of hard bounces """
import time

from notifications_utils.clients.redis.redis_client import RedisClient


def _hard_bounce_key(service_id: str):
def hard_bounce_key(service_id: str):
return f"sliding_hard_bounce:{service_id}"


def _notifications_key(service_id: str):
return f"sliding_notifications:{service_id}"


def _total_notifications_seeded_key(service_id: str):
return f"total_notifications_seeded:{service_id}"


def _total_hard_bounces_seeded_key(service_id: str):
return f"total_hard_bounces_seeded:{service_id}"
def total_notifications_key(service_id: str):
return f"sliding_total_notifications:{service_id}"


def _twenty_four_hour_window():
return 60 * 60 * 24
def _twenty_four_hour_window_ms():
return 60 * 60 * 24 * 1000


def _current_time():
return int(time.time())
def _current_time_ms():
return int(time.time()) * 1000


class RedisBounceRate:
def __init__(self, redis=RedisClient()):
def __init__(self, redis):
self._redis_client = redis

def set_sliding_hard_bounce(self, service_id: str):
current_time = _current_time()
self._redis_client.add_key_to_sorted_set(_hard_bounce_key(service_id), current_time, current_time)

def set_sliding_notifications(self, service_id: str):
current_time = _current_time()
self._redis_client.add_key_to_sorted_set(_notifications_key(service_id), current_time, current_time)
current_time = _current_time_ms()
self._redis_client.add_key_to_sorted_set(total_notifications_key(service_id), {current_time: current_time})

def set_total_notifications_seeded(self, service_id: str, time_of_bounce, value):
# Strip the time down to the hour and convert to epoch
bounce_epoch = time_of_bounce.replace(minute=0, second=0, microsecond=0).timestamp()
cache_key = _total_notifications_seeded_key(service_id)
self._redis_client.add_key_to_sorted_set(cache_key, bounce_epoch, value)
self._redis_client.expire(cache_key, _twenty_four_hour_window())
def set_sliding_hard_bounce(self, service_id: str):
current_time = _current_time_ms()
self._redis_client.add_key_to_sorted_set(hard_bounce_key(service_id), {current_time: current_time})

def set_total_hard_bounce_seeded(self, service_id: str, time_of_bounce, value):
# Strip the time down to the hour and convert to epoch
bounce_epoch = time_of_bounce.replace(minute=0, second=0, microsecond=0).timestamp()
cache_key = _total_hard_bounces_seeded_key(service_id)
self._redis_client.add_key_to_sorted_set(cache_key, bounce_epoch, value)
self._redis_client.expire(cache_key, _twenty_four_hour_window())
def set_notifications_seeded(self, service_id: str, seeded_data: dict):
self._redis_client.add_key_to_sorted_set(total_notifications_key(service_id), seeded_data)

def get_bounce_rate(self, service_id: str, bounce_window=_twenty_four_hour_window()) -> int:
total_hard_bounces_sliding = self._redis_client.get_length_of_sorted_set(_hard_bounce_key(service_id), bounce_window)
total_notifications_sliding = self._redis_client.get_length_of_sorted_set(_notifications_key(service_id), bounce_window)
total_hard_bounces_seeded = self._redis_client.get_sorted_set_members_by_score(
_total_hard_bounces_seeded_key(service_id), _current_time() - bounce_window, _current_time()
)
total_notifications_seeded = self._redis_client.get_sorted_set_members_by_score(
_total_notifications_seeded_key(service_id), _current_time() - bounce_window, _current_time()
def set_hard_bounce_seeded(self, service_id: str, seeded_data: dict):
self._redis_client.add_key_to_sorted_set(hard_bounce_key(service_id), seeded_data)

def get_bounce_rate(self, service_id: str, bounce_window=_twenty_four_hour_window_ms()) -> int:
total_hard_bounces_sliding = self._redis_client.get_length_of_sorted_set(hard_bounce_key(service_id), bounce_window)
total_notifications_sliding = self._redis_client.get_length_of_sorted_set(
total_notifications_key(service_id), bounce_window
)
return (
round(
(total_hard_bounces_sliding + total_hard_bounces_seeded)
/ (total_notifications_sliding + total_notifications_seeded),
2,
)
if (total_notifications_sliding + total_notifications_seeded > 0)
else 0
round(total_hard_bounces_sliding / (1.0 * total_notifications_sliding), 2) if (total_notifications_sliding > 0) else 0
)
14 changes: 6 additions & 8 deletions notifications_utils/clients/redis/redis_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,19 +129,17 @@ def exceeded_rate_limit(self, cache_key, limit, interval, raise_exception=False)
else:
return False

def add_key_to_sorted_set(self, cache_key, score, value, raise_exception=False):
def add_key_to_sorted_set(self, cache_key: str, sorted_set_data: dict, raise_exception=False) -> None:
"""
Add a key to a sorted set, with a score
:param cache_key:
:param value:
:param score:
:param raise_exception:
Add data to a sorted set
:param cache_key: the redis key for the sorted set to add to
:param sorted_set_data: the data to add to the sorted set, in the form {key1: score1, key2: score2}
:param raise_exception: True if we should allow the exception to bubble up
"""
cache_key = prepare_value(cache_key)
value = prepare_value(value)
if self.active:
try:
self.redis_store.zadd(cache_key, {score: value})
self.redis_store.zadd(cache_key, sorted_set_data)
except Exception as e:
self.__handle_exception(e, raise_exception, "add-key-to-ordered-set", cache_key)

Expand Down
54 changes: 18 additions & 36 deletions tests/test_bounce_rate.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,14 @@
import datetime
import random
import uuid
import pytest
from unittest.mock import Mock
from freezegun import freeze_time

from notifications_utils.clients.redis.bounce_rate import (
_current_time,
_current_time_ms,
RedisBounceRate,
_hard_bounce_key,
_notifications_key,
_total_notifications_seeded_key,
_total_hard_bounces_seeded_key,
hard_bounce_key,
total_notifications_key,
)
from notifications_utils.clients.redis.redis_client import RedisClient

Expand Down Expand Up @@ -50,12 +47,7 @@ def mocked_seeded_data_hours():
def build_bounce_rate_client(mocker, mocked_redis_client):
bounce_rate_client = RedisBounceRate(mocked_redis_client)
mocker.patch.object(bounce_rate_client._redis_client, "add_key_to_sorted_set")
mocker.patch.object(
bounce_rate_client._redis_client, "get_length_of_sorted_set", side_effect=[8, 20, 0, 0, 0, 8, 0, 0, 10, 20]
)
mocker.patch.object(
bounce_rate_client._redis_client, "get_sorted_set_members_by_score", side_effect=[0, 0, 1, 2, 0, 0, 0, 0, 4, 8]
)
mocker.patch.object(bounce_rate_client._redis_client, "get_length_of_sorted_set", side_effect=[8, 20, 0, 0, 0, 8, 10, 20])
mocker.patch.object(bounce_rate_client._redis_client, "expire")
return bounce_rate_client

Expand All @@ -70,24 +62,21 @@ class TestRedisBounceRate:
def test_set_hard_bounce(self, mocked_bounce_rate_client, mocked_service_id):
mocked_bounce_rate_client.set_sliding_hard_bounce(mocked_service_id)
mocked_bounce_rate_client._redis_client.add_key_to_sorted_set.assert_called_with(
_hard_bounce_key(mocked_service_id), _current_time(), _current_time()
hard_bounce_key(mocked_service_id), {_current_time_ms(): _current_time_ms()}
)

@freeze_time("2001-01-01 12:00:00.000000")
def test_set_total_notifications(self, mocked_bounce_rate_client, mocked_service_id):
mocked_bounce_rate_client.set_sliding_notifications(mocked_service_id)
mocked_bounce_rate_client._redis_client.add_key_to_sorted_set.assert_called_with(
_notifications_key(mocked_service_id), _current_time(), _current_time()
total_notifications_key(mocked_service_id), {_current_time_ms(): _current_time_ms()}
)

@freeze_time("2001-01-01 12:00:00.000000")
def test_get_bounce_rate(self, mocked_bounce_rate_client, mocked_service_id):
answer = mocked_bounce_rate_client.get_bounce_rate(mocked_service_id)
assert answer == 0.4

answer = mocked_bounce_rate_client.get_bounce_rate(mocked_service_id)
assert answer == 0.5

answer = mocked_bounce_rate_client.get_bounce_rate(mocked_service_id)
assert answer == 0

Expand All @@ -97,27 +86,20 @@ def test_get_bounce_rate(self, mocked_bounce_rate_client, mocked_service_id):
answer = mocked_bounce_rate_client.get_bounce_rate(mocked_service_id)
assert answer == 0.5

def test_set_total_hard_bounce_seeded_with_24_hour_period(
self, mocked_bounce_rate_client, mocked_service_id, mocked_seeded_data_hours
def test_set_total_hard_bounce_seeded(
self,
mocked_bounce_rate_client,
mocked_service_id,
):
for hour in mocked_seeded_data_hours:
bounce_count = random.randint(1, 10)
bounce_epoch = hour.replace(minute=0, second=0, microsecond=0).timestamp()
mocked_bounce_rate_client.set_total_hard_bounce_seeded(mocked_service_id, hour, bounce_count)
mocked_bounce_rate_client._redis_client.add_key_to_sorted_set.assert_called_with(
_total_hard_bounces_seeded_key(mocked_service_id), bounce_epoch, bounce_count
)
mocked_bounce_rate_client._redis_client.expire.assert_called_with(
_total_hard_bounces_seeded_key(mocked_service_id), 60 * 60 * 24
)
seeded_data = {12345: 12345, 12346: 12346}
mocked_bounce_rate_client.set_hard_bounce_seeded(mocked_service_id, seeded_data)
mocked_bounce_rate_client._redis_client.add_key_to_sorted_set.assert_called_with(
hard_bounce_key(mocked_service_id), seeded_data
)

def test_set_total_notifications_seeded(self, mocked_bounce_rate_client, mocked_service_id):
current_time = datetime.datetime.now()
bounce_epoch = current_time.replace(minute=0, second=0, microsecond=0).timestamp()
mocked_bounce_rate_client.set_total_notifications_seeded(mocked_service_id, current_time, 20)
seeded_data = {12345: 12345, 12346: 12346}
mocked_bounce_rate_client.set_notifications_seeded(mocked_service_id, seeded_data)
mocked_bounce_rate_client._redis_client.add_key_to_sorted_set.assert_called_with(
_total_notifications_seeded_key(mocked_service_id), bounce_epoch, 20
)
mocked_bounce_rate_client._redis_client.expire.assert_called_with(
_total_notifications_seeded_key(mocked_service_id), 60 * 60 * 24
total_notifications_key(mocked_service_id), seeded_data
)
2 changes: 1 addition & 1 deletion tests/test_redis_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ def test_delete_cache_keys_returns_zero_when_redis_disabled(mocked_redis_client)

class TestRedisSortedSets:
def test_add_to_redis_sorted_set(self, mocked_redis_client):
mocked_redis_client.add_key_to_sorted_set("key", "value", 1)
mocked_redis_client.add_key_to_sorted_set("key", {"value": 1})
mocked_redis_client.redis_store.zadd.assert_called_with("key", {"value": 1})

def test_delete_from_redis_sorted_set(self, mocked_redis_client):
Expand Down

0 comments on commit 80fd385

Please sign in to comment.