Skip to content

Commit

Permalink
Merge pull request #74 from codecov/scott/status-caching
Browse files Browse the repository at this point in the history
feat: Only send status checks when payload has changed
  • Loading branch information
scott-codecov committed Aug 29, 2023
2 parents aca010e + 348d638 commit 569d4ee
Show file tree
Hide file tree
Showing 2 changed files with 118 additions and 1 deletion.
44 changes: 43 additions & 1 deletion services/notification/notifiers/status/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
from typing import Dict

from shared.analytics_tracking import track_event
from shared.config import get_config
from shared.torngit.exceptions import TorngitClientError, TorngitError
from shared.utils.sessions import SessionType

from helpers.cache import DEFAULT_TTL, NO_VALUE, cache, make_hash_sha256
from helpers.environment import is_enterprise
from helpers.match import match
from helpers.metrics import metrics
Expand Down Expand Up @@ -209,7 +211,47 @@ async def notify(self, comparison: Comparison):
payload["url"] = get_pull_url(comparison.pull)
else:
payload["url"] = get_commit_url(comparison.head.commit)
return await self.send_notification(comparison, payload)

base_commit = comparison.base.commit if comparison.base else None
head_commit = comparison.head.commit if comparison.head else None

cache_key = make_hash_sha256(
dict(
type="status_check_notification",
repoid=head_commit.repoid,
base_commitid=base_commit.commitid if base_commit else None,
head_commitid=head_commit.commitid if head_commit else None,
notifier_name=self.name,
notifier_title=self.title,
)
)

last_payload = cache.get_backend().get(cache_key)
if last_payload is NO_VALUE or last_payload != payload:
ttl = int(
get_config(
"setup", "cache", "send_status_notification", default=600
)
) # 10 min default
cache.get_backend().set(cache_key, ttl, payload)
return await self.send_notification(comparison, payload)
else:
log.info(
"Notification payload unchanged. Skipping notification.",
extra=dict(
repoid=head_commit.repoid,
base_commitid=base_commit.commitid if base_commit else None,
head_commitid=head_commit.commitid if head_commit else None,
notifier_name=self.name,
notifier_title=self.title,
),
)
return NotificationResult(
notification_attempted=False,
notification_successful=None,
explanation="payload_unchanged",
data_sent=None,
)
except TorngitClientError:
log.warning(
"Unable to send status notification to user due to a client-side error",
Expand Down
75 changes: 75 additions & 0 deletions services/notification/notifiers/tests/unit/test_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
ProjectStatusNotifier,
)
from services.notification.notifiers.status.base import StatusNotifier
from services.urls import get_pull_url


def test_notification_type(mocker):
Expand Down Expand Up @@ -487,6 +488,80 @@ async def test_notify_no_base(
}
assert result.data_received == {"id": "some_id"}

@pytest.mark.asyncio
async def test_notify_uncached(
self,
sample_comparison,
mocker,
):
comparison = sample_comparison
payload = {
"message": "something to say",
"state": "success",
"url": get_pull_url(comparison.pull),
}

class TestNotifier(StatusNotifier):
async def build_payload(self, comparison):
return payload

notifier = TestNotifier(
repository=comparison.head.commit.repository,
title="title",
notifier_yaml_settings={},
notifier_site_settings=True,
current_yaml=UserYaml({}),
)
notifier.context = "fake"

send_notification = mocker.patch.object(TestNotifier, "send_notification")
await notifier.notify(comparison)
send_notification.assert_called_once

@pytest.mark.asyncio
async def test_notify_cached(
self,
sample_comparison,
mocker,
):
comparison = sample_comparison

payload = {
"message": "something to say",
"state": "success",
"url": get_pull_url(comparison.pull),
}

class TestNotifier(StatusNotifier):
async def build_payload(self, comparison):
return payload

notifier = TestNotifier(
repository=comparison.head.commit.repository,
title="title",
notifier_yaml_settings={},
notifier_site_settings=True,
current_yaml=UserYaml({}),
)
notifier.context = "fake"

mocker.patch(
"helpers.cache.NullBackend.get",
return_value=payload,
)

send_notification = mocker.patch.object(TestNotifier, "send_notification")
result = await notifier.notify(comparison)
assert result == NotificationResult(
notification_attempted=False,
notification_successful=None,
explanation="payload_unchanged",
data_sent=None,
)

# payload was cached - we do not send the notification
assert not send_notification.called

@pytest.mark.asyncio
async def test_send_notification(
self, sample_comparison, mocker, mock_repo_provider
Expand Down

0 comments on commit 569d4ee

Please sign in to comment.