Skip to content

Commit

Permalink
fix(webhooks): default task processor to use processor and prevent we…
Browse files Browse the repository at this point in the history
…bhook retries in non-processor environments (#3273)
  • Loading branch information
matthewelwell authored Jan 11, 2024
1 parent c87749e commit 4d002fc
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 5 deletions.
12 changes: 9 additions & 3 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 All @@ -897,7 +895,11 @@
# Setting to allow asynchronous tasks to be run synchronously for testing purposes
# or in a separate thread for self-hosted users
TASK_RUN_METHOD = env.enum(
"TASK_RUN_METHOD", type=TaskRunMethod, default=TaskRunMethod.SEPARATE_THREAD.value
"TASK_RUN_METHOD",
type=TaskRunMethod,
default=TaskRunMethod.TASK_PROCESSOR.value
if env.bool("RUN_BY_PROCESSOR", False)
else TaskRunMethod.SEPARATE_THREAD.value,
)
ENABLE_TASK_PROCESSOR_HEALTH_CHECK = env.bool(
"ENABLE_TASK_PROCESSOR_HEALTH_CHECK", default=False
Expand All @@ -915,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

3 comments on commit 4d002fc

@vercel
Copy link

@vercel vercel bot commented on 4d002fc Jan 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

docs – ./docs

docs.flagsmith.com
docs-flagsmith.vercel.app
docs.bullet-train.io
docs-git-main-flagsmith.vercel.app

@vercel
Copy link

@vercel vercel bot commented on 4d002fc Jan 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vercel
Copy link

@vercel vercel bot commented on 4d002fc Jan 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.