Skip to content

Commit

Permalink
fix(webhooks): prevent raise on give up (#3295)
Browse files Browse the repository at this point in the history
  • Loading branch information
matthewelwell authored Jan 16, 2024
1 parent a54352f commit 581a8c9
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 67 deletions.
4 changes: 2 additions & 2 deletions api/integrations/rudderstack/rudderstack.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import logging
import typing

import rudder_analytics
from rudderstack import analytics as rudder_analytics

from environments.identities.models import Identity
from environments.identities.traits.models import Trait
Expand All @@ -16,7 +16,7 @@
class RudderstackWrapper(AbstractBaseIdentityIntegrationWrapper):
def __init__(self, config: RudderstackConfiguration):
rudder_analytics.write_key = config.api_key
rudder_analytics.data_plane_url = config.base_url
rudder_analytics.dataPlaneUrl = config.base_url

def _identify_user(self, user_data: dict) -> None:
rudder_analytics.identify(**user_data)
Expand Down
118 changes: 56 additions & 62 deletions api/poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions api/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ readme = "readme.md"
[tool.poetry.dependencies]
python = ">=3.10,<3.12"
django = "~3.2.23"
rudder-sdk-python = "~1.0.5"
analytics-python = "~1.4.0"
backoff = "~1.10.0"
rudder-sdk-python = "~2.0.2"
segment-analytics-python = "~2.2.3"
backoff = "~2.2.1"
appdirs = "~1.4.4"
django-cors-headers = "~3.5.0"
djangorestframework = "~3.12.1"
Expand Down
26 changes: 26 additions & 0 deletions api/tests/unit/webhooks/test_unit_webhooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from unittest import TestCase, mock

import pytest
import responses
from core.constants import FLAGSMITH_SIGNATURE_HEADER
from pytest_django.fixtures import SettingsWrapper
from pytest_mock import MockerFixture
Expand All @@ -21,6 +22,7 @@
WebhookEventType,
WebhookType,
call_environment_webhooks,
call_integration_webhook,
call_organisation_webhooks,
call_webhook_with_failure_mail_after_retries,
trigger_sample_webhook,
Expand Down Expand Up @@ -316,3 +318,27 @@ def test_call_webhook_with_failure_mail_after_retries_does_not_retry_if_not_usin
# Then
assert requests_post_mock.call_count == 1
send_failure_email_mock.assert_called_once()


@responses.activate()
def test_call_integration_webhook_does_not_raise_error_on_backoff_give_up(
mocker: MockerFixture,
) -> None:
"""
This test is essentially verifying that the `raise_on_giveup` argument
passed to the backoff decorator on _call_webhook is working as we
expect it to.
"""
# Given
url = "https://test.com/webhook"
config = mocker.MagicMock(secret=None, url=url)

responses.add(url=url, method="POST", body=json.dumps({}), status=400)

# When
result = call_integration_webhook(config, data={})

# Then
# we don't get a result from the function (as expected), and no exception is
# raised
assert result is None
2 changes: 2 additions & 0 deletions api/webhooks/webhooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ def trigger_sample_webhook(
wait_gen=backoff.expo,
exception=requests.exceptions.RequestException,
max_tries=settings.WEBHOOK_BACKOFF_RETRIES,
raise_on_giveup=False,
giveup_log_level=logging.WARNING,
)
def _call_webhook(
webhook: AbstractBaseWebhookModel,
Expand Down

3 comments on commit 581a8c9

@vercel
Copy link

@vercel vercel bot commented on 581a8c9 Jan 16, 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 581a8c9 Jan 16, 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 581a8c9 Jan 16, 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-git-main-flagsmith.vercel.app
docs-flagsmith.vercel.app
docs.flagsmith.com
docs.bullet-train.io

Please sign in to comment.