Skip to content

Commit

Permalink
Prevent webhook retries if not using task processor
Browse files Browse the repository at this point in the history
  • Loading branch information
matthewelwell committed Jan 11, 2024
1 parent 70a4eaa commit 8a6246f
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 4 deletions.
6 changes: 4 additions & 2 deletions api/app/settings/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -879,8 +879,6 @@
and EDGE_RELEASE_DATETIME < datetime.now(tz=pytz.UTC)
)

DISABLE_WEBHOOKS = env.bool("DISABLE_WEBHOOKS", False)

DISABLE_FLAGSMITH_UI = env.bool("DISABLE_FLAGSMITH_UI", default=False)
SERVE_FE_ASSETS = not DISABLE_FLAGSMITH_UI and os.path.exists(
BASE_DIR + "/app/templates/webpack/index.html"
Expand Down Expand Up @@ -919,6 +917,10 @@
"RECURRING_TASK_RUN_RETENTION_DAYS", default=30
)

# Webhook settings
DISABLE_WEBHOOKS = env.bool("DISABLE_WEBHOOKS", False)
RETRY_WEBHOOKS = TASK_RUN_METHOD == TaskRunMethod.TASK_PROCESSOR

# Real time(server sent events) settings
SSE_SERVER_BASE_URL = env.str("SSE_SERVER_BASE_URL", None)
SSE_AUTHENTICATION_TOKEN = env.str("SSE_AUTHENTICATION_TOKEN", None)
Expand Down
2 changes: 2 additions & 0 deletions api/app/settings/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,5 @@
}

AWS_SSE_LOGS_BUCKET_NAME = "test_bucket"

RETRY_WEBHOOKS = True
36 changes: 35 additions & 1 deletion api/tests/unit/webhooks/test_unit_webhooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import pytest
from core.constants import FLAGSMITH_SIGNATURE_HEADER
from pytest_django.fixtures import SettingsWrapper
from pytest_mock import MockerFixture
from requests.exceptions import ConnectionError, Timeout

Expand Down Expand Up @@ -223,7 +224,10 @@ def test_call_environment_webhooks__multiple_webhooks__failure__calls_expected(
@pytest.mark.parametrize("expected_error", [ConnectionError, Timeout])
@pytest.mark.django_db
def test_call_organisation_webhooks__multiple_webhooks__failure__calls_expected(
mocker: MockerFixture, expected_error: Type[Exception], organisation: Organisation
mocker: MockerFixture,
expected_error: Type[Exception],
organisation: Organisation,
settings: SettingsWrapper,
) -> None:
# Given
requests_post_mock = mocker.patch("webhooks.webhooks.requests.post")
Expand All @@ -248,6 +252,7 @@ def test_call_organisation_webhooks__multiple_webhooks__failure__calls_expected(
expected_send_failure_status_code = f"N/A ({expected_error.__name__})"

retries = 5

# When
call_organisation_webhooks(
organisation_id=organisation.id,
Expand Down Expand Up @@ -282,3 +287,32 @@ def test_call_webhook_with_failure_mail_after_retries_raises_error_on_invalid_ar
try_count = 10
with pytest.raises(ValueError):
call_webhook_with_failure_mail_after_retries(0, {}, "", try_count=try_count)


def test_call_webhook_with_failure_mail_after_retries_does_not_retry_if_not_using_processor(
mocker: MockerFixture, organisation: Organisation, settings: SettingsWrapper
):
# Given
requests_post_mock = mocker.patch("webhooks.webhooks.requests.post")
requests_post_mock.side_effect = ConnectionError
send_failure_email_mock: mock.Mock = mocker.patch(
"webhooks.webhooks.send_failure_email"
)

settings.RETRY_WEBHOOKS = False

webhook = OrganisationWebhook.objects.create(
url="http://url.1.com", enabled=True, organisation=organisation
)

# When
call_webhook_with_failure_mail_after_retries(
webhook.id,
data={},
webhook_type=WebhookType.ORGANISATION.value,
send_failure_mail=True,
)

# Then
assert requests_post_mock.call_count == 1
send_failure_email_mock.assert_called_once()
2 changes: 1 addition & 1 deletion api/webhooks/webhooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ def call_webhook_with_failure_mail_after_retries(
)
res.raise_for_status()
except requests.exceptions.RequestException as exc:
if try_count == max_retries:
if try_count == max_retries or not settings.RETRY_WEBHOOKS:
if send_failure_mail:
send_failure_email(
webhook,
Expand Down

0 comments on commit 8a6246f

Please sign in to comment.