Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Call webhooks async and add backoff to webhooks #2932

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
8d4d03e
test
tushar5526 Jul 31, 2023
80cced0
feat: Add backoff to webhooks, and call multiple webhooks async
tushar5526 Aug 1, 2023
72f4829
Merge branch 'main' of https://github.com/flagsmith/flagsmith
tushar5526 Aug 19, 2023
49c1f89
feat: Add backoff to webhooks, and call multiple webhooks async
tushar5526 Jul 31, 2023
5e476a7
feat: updated tests
tushar5526 Aug 19, 2023
6a18e08
fix: Update tests to support webhooks using task processor
tushar5526 Aug 19, 2023
ee1c87b
fix: Update tests to use delay in call_webhooks
tushar5526 Aug 20, 2023
d6bc569
fix: don't call webhooks if object is deleted
tushar5526 Aug 21, 2023
f99c140
fix type
tushar5526 Aug 21, 2023
b0be5f6
feat: add type hinting and remove print line
tushar5526 Aug 21, 2023
6a218fe
fix: fix type hints
tushar5526 Aug 21, 2023
a224cdc
Merge branch 'Flagsmith:main' into webhook
tushar5526 Sep 5, 2023
f24fe47
chore: Make _call_webhook_email_on_error public function
tushar5526 Sep 6, 2023
8d22980
fix: Pass webhook objects instead of ids
tushar5526 Sep 6, 2023
c4f99ee
Merge branch 'main' into webhook
tushar5526 Sep 8, 2023
72b1676
fix: Cap max_time for backoffs to run to 2s, so the thread remains he…
tushar5526 Sep 8, 2023
fdba296
feat: update tests to add in stricter checks
tushar5526 Sep 12, 2023
077bfb1
Merge branch 'main' into webhook
tushar5526 Sep 15, 2023
9a41ff7
feat: Add backoff to webhooks
tushar5526 Nov 6, 2023
304268e
Merge branch 'main' into fix/1654/webhooks-backoff-features
tushar5526 Nov 6, 2023
c61220c
feat: add tests for organisation webhook
tushar5526 Nov 6, 2023
d6a5c46
tests: add tests for args check
tushar5526 Nov 6, 2023
2a6e428
feat: update tests and move webhook constants to common settings
tushar5526 Nov 11, 2023
2cf8703
fix: raise error status on bad status codes
tushar5526 Nov 11, 2023
d860c71
Merge branch 'main' into fix/1654/webhooks-backoff-features
tushar5526 Nov 16, 2023
3933ce3
refactor tests and impelment suggestion from reviews
tushar5526 Nov 16, 2023
3d66982
update unit test cases
tushar5526 Nov 16, 2023
a165e9d
Add backoff and typing to integration webhook function
tushar5526 Nov 16, 2023
ffebf6a
Update api/webhooks/webhooks.py
tushar5526 Nov 17, 2023
3d7cedd
fix: Fix typing issue
tushar5526 Nov 17, 2023
7ea455c
Merge branch 'main' into fix/1654/webhooks-backoff-features
tushar5526 Nov 20, 2023
16546ea
Merge branch 'main' into fix/1654/webhooks-backoff-features
tushar5526 Nov 20, 2023
1f71356
Merge branch 'main' into fix/1654/webhooks-backoff-features
tushar5526 Nov 22, 2023
9c4e1ec
Merge branch 'main' into fix/1654/webhooks-backoff-features
tushar5526 Nov 24, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions api/app/settings/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -1046,3 +1046,6 @@
# The LDAP user username and password used by `sync_ldap_users_and_groups` command
LDAP_SYNC_USER_USERNAME = env.str("LDAP_SYNC_USER_USERNAME", None)
LDAP_SYNC_USER_PASSWORD = env.str("LDAP_SYNC_USER_PASSWORD", None)

WEBHOOK_BACKOFF_BASE = env.int("WEBHOOK_BACKOFF_BASE", default=2)
WEBHOOK_BACKOFF_RETRIES = env.int("WEBHOOK_BACKOFF_RETRIES", default=3)
4 changes: 3 additions & 1 deletion api/audit/signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ def call_webhooks(sender, instance, **kwargs):
if instance.project
else instance.environment.project.organisation
)
call_organisation_webhooks(organisation, data, WebhookEventType.AUDIT_LOG_CREATED)
call_organisation_webhooks.delay(
args=(organisation.id, data, WebhookEventType.AUDIT_LOG_CREATED.value)
)


def _get_integration_config(instance, integration_name):
Expand Down
2 changes: 1 addition & 1 deletion api/edge_api/identities/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def call_environment_webhook_for_feature_state_change(
else WebhookEventType.FLAG_UPDATED
)

call_environment_webhooks(environment, data, event_type=event_type)
call_environment_webhooks(environment.id, data, event_type=event_type.value)


@register_task_handler(priority=TaskPriority.HIGH)
Expand Down
21 changes: 9 additions & 12 deletions api/features/tasks.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
from threading import Thread

from environments.models import Webhook
from features.models import FeatureState
from webhooks.constants import WEBHOOK_DATETIME_FORMAT
Expand Down Expand Up @@ -38,19 +36,18 @@ def trigger_feature_state_change_webhooks(
previous_state = _get_previous_state(history_instance, event_type)
if previous_state:
data.update(previous_state=previous_state)
Thread(
target=call_environment_webhooks,
args=(instance.environment, data, event_type),
).start()

Thread(
target=call_organisation_webhooks,
call_environment_webhooks.delay(
args=(instance.environment.id, data, event_type.value)
)

call_organisation_webhooks.delay(
args=(
instance.environment.project.organisation,
instance.environment.project.organisation.id,
data,
event_type,
),
).start()
event_type.value,
)
)


def _get_previous_state(
Expand Down
2 changes: 1 addition & 1 deletion api/task_processor/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def delay(
delay_until: datetime | None = None,
# TODO @khvn26 consider typing `args` and `kwargs` with `ParamSpec`
# (will require a change to the signature)
args: tuple[typing.Any] = (),
args: tuple[typing.Any, ...] = (),
kwargs: dict[str, typing.Any] | None = None,
) -> Task | None:
logger.debug("Request to run task '%s' asynchronously.", self.task_identifier)
Expand Down
10 changes: 9 additions & 1 deletion api/tests/unit/audit/test_unit_audit_models.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from audit.models import AuditLog
from audit.related_object_type import RelatedObjectType
from audit.serializers import AuditLogSerializer
from integrations.datadog.models import DataDogConfiguration
from webhooks.webhooks import WebhookEventType


def test_organisation_webhooks_are_called_when_audit_log_saved(project, mocker):
Expand All @@ -13,7 +15,13 @@ def test_organisation_webhooks_are_called_when_audit_log_saved(project, mocker):
audit_log.save()

# Then
mock_call_webhooks.assert_called()
mock_call_webhooks.delay.assert_called_once_with(
args=(
project.organisation.id,
AuditLogSerializer(instance=audit_log).data,
WebhookEventType.AUDIT_LOG_CREATED.value,
)
)


def test_data_dog_track_event_not_called_on_audit_log_saved_when_not_configured(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ def test_call_environment_webhook_for_feature_state_change_with_new_state_only(
mock_call_environment_webhooks.assert_called_once()
call_args = mock_call_environment_webhooks.call_args

assert call_args[0][0] == environment
assert call_args[1]["event_type"] == WebhookEventType.FLAG_UPDATED
assert call_args[0][0] == environment.id
assert call_args[1]["event_type"] == WebhookEventType.FLAG_UPDATED.value

mock_generate_webhook_feature_state_data.assert_called_once_with(
feature=feature,
Expand Down Expand Up @@ -105,8 +105,8 @@ def test_call_environment_webhook_for_feature_state_change_with_previous_state_o
mock_call_environment_webhooks.assert_called_once()
call_args = mock_call_environment_webhooks.call_args

assert call_args[0][0] == environment
assert call_args[1]["event_type"] == WebhookEventType.FLAG_DELETED
assert call_args[0][0] == environment.id
assert call_args[1]["event_type"] == WebhookEventType.FLAG_DELETED.value

mock_generate_webhook_feature_state_data.assert_called_once_with(
feature=feature,
Expand Down Expand Up @@ -176,8 +176,8 @@ def test_call_environment_webhook_for_feature_state_change_with_both_states(
mock_call_environment_webhooks.assert_called_once()
call_args = mock_call_environment_webhooks.call_args

assert call_args[0][0] == environment
assert call_args[1]["event_type"] == WebhookEventType.FLAG_UPDATED
assert call_args[0][0] == environment.id
assert call_args[1]["event_type"] == WebhookEventType.FLAG_UPDATED.value

assert mock_generate_webhook_feature_state_data.call_count == 2
mock_generate_data_calls = mock_generate_webhook_feature_state_data.call_args_list
Expand Down
67 changes: 38 additions & 29 deletions api/tests/unit/features/test_unit_features_tasks.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from unittest import mock

import pytest
from pytest_mock import MockerFixture

from environments.models import Environment
from features.models import Feature, FeatureState
Expand All @@ -11,8 +10,7 @@


@pytest.mark.django_db
@mock.patch("features.tasks.Thread")
def test_trigger_feature_state_change_webhooks(MockThread):
def test_trigger_feature_state_change_webhooks(mocker: MockerFixture):
# Given
initial_value = "initial"
new_value = "new"
Expand All @@ -30,34 +28,40 @@ def test_trigger_feature_state_change_webhooks(MockThread):
feature_state.feature_state_value.save()
feature_state.save()

MockThread.reset_mock() # reset mock as it will have been called when setting up the data
mock_call_environment_webhooks = mocker.patch(
"features.tasks.call_environment_webhooks"
)
mock_call_organisation_webhooks = mocker.patch(
"features.tasks.call_organisation_webhooks"
)

# When
trigger_feature_state_change_webhooks(feature_state)

# Then
call_list = MockThread.call_args_list
environment_webhook_call_args = (
mock_call_environment_webhooks.delay.call_args.kwargs["args"]
)
organisation_webhook_call_args = (
mock_call_organisation_webhooks.delay.call_args.kwargs["args"]
)

environment_webhook_call_args = call_list[0]
organisation_webhook_call_args = call_list[1]
assert environment_webhook_call_args[0] == environment.id
assert organisation_webhook_call_args[0] == organisation.id

# verify that the data for both calls is the same
assert (
environment_webhook_call_args[1]["args"][1]
== organisation_webhook_call_args[1]["args"][1]
)
assert environment_webhook_call_args[1] == organisation_webhook_call_args[1]

data = environment_webhook_call_args[1]["args"][1]
event_type = environment_webhook_call_args[1]["args"][2]
data = environment_webhook_call_args[1]
event_type = environment_webhook_call_args[2]
assert data["new_state"]["feature_state_value"] == new_value
assert data["previous_state"]["feature_state_value"] == initial_value
assert event_type == WebhookEventType.FLAG_UPDATED
assert event_type == WebhookEventType.FLAG_UPDATED.value


@pytest.mark.django_db
@mock.patch("features.tasks.Thread")
def test_trigger_feature_state_change_webhooks_for_deleted_flag(
MockThread, organisation, project, environment, feature
mocker, organisation, project, environment, feature
):
# Given
new_value = "new"
Expand All @@ -68,23 +72,28 @@ def test_trigger_feature_state_change_webhooks_for_deleted_flag(
feature_state.feature_state_value.save()
feature_state.save()

MockThread.reset_mock() # reset mock as it will have been called when setting up the data
mock_call_environment_webhooks = mocker.patch(
"features.tasks.call_environment_webhooks"
)
mock_call_organisation_webhooks = mocker.patch(
"features.tasks.call_organisation_webhooks"
)

trigger_feature_state_change_webhooks(feature_state, WebhookEventType.FLAG_DELETED)

# Then
call_list = MockThread.call_args_list

environment_webhook_call_args = call_list[0]
organisation_webhook_call_args = call_list[1]
environment_webhook_call_args = (
mock_call_environment_webhooks.delay.call_args.kwargs["args"]
)
organisation_webhook_call_args = (
mock_call_organisation_webhooks.delay.call_args.kwargs["args"]
)

# verify that the data for both calls is the same
assert (
environment_webhook_call_args[1]["args"][1]
== organisation_webhook_call_args[1]["args"][1]
)
assert environment_webhook_call_args[1] == organisation_webhook_call_args[1]

data = environment_webhook_call_args[1]["args"][1]
event_type = environment_webhook_call_args[1]["args"][2]
data = environment_webhook_call_args[1]
event_type = environment_webhook_call_args[2]
assert data["new_state"] is None
assert data["previous_state"]["feature_state_value"] == new_value
assert event_type == WebhookEventType.FLAG_DELETED
assert event_type == WebhookEventType.FLAG_DELETED.value
Loading