Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
10 changes: 6 additions & 4 deletions src/sentry/workflow_engine/models/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,10 @@ def get_snapshot(self) -> WorkflowSnapshot:
}

def evaluate_trigger_conditions(
self, event_data: WorkflowEventData, when_data_conditions: list[DataCondition] | None = None
self,
event_data: WorkflowEventData,
when_data_conditions: list[DataCondition] | None = None,
group: DataConditionGroup | None = None,
) -> tuple[TriggerResult, list[DataCondition]]:
"""
Evaluate the conditions for the workflow trigger and return if the evaluation was successful.
Expand All @@ -140,9 +143,8 @@ def evaluate_trigger_conditions(
return TriggerResult.TRUE, []

workflow_event_data = replace(event_data, workflow_env=self.environment)
try:
group = DataConditionGroup.objects.get_from_cache(id=self.when_condition_group_id)
except DataConditionGroup.DoesNotExist:

if group is None:
Comment thread
cmanallen marked this conversation as resolved.
# This isn't expected under normal conditions, but weird things can happen in the
# midst of deletions and migrations.
logger.exception(
Expand Down
9 changes: 8 additions & 1 deletion src/sentry/workflow_engine/processors/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,14 +156,21 @@ def evaluate_workflow_triggers(
# Retrieve these as a batch to avoid a query/cache-lookup per DCG.
data_conditions_by_dcg_id = _get_data_conditions_for_group_by_dcg(dcg_ids)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can we not use these we've already fetched?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That caches list[DataConditions]. The new variable caches DataConditionsGroup.


# Retrieve data condition groups as a batch to avoid a query/cache-lookup per DCG.
data_condition_groups_by_id: dict[int, DataConditionGroup] = {
dcg.id: dcg for dcg in DataConditionGroup.objects.get_many_from_cache(dcg_ids)
}

tainted_untriggered, untainted_untriggered = 0, 0
for workflow in workflows:
when_data_conditions = None
when_condition_group = None
if dcg_id := workflow.when_condition_group_id:
when_data_conditions = data_conditions_by_dcg_id.get(dcg_id)
when_condition_group = data_condition_groups_by_id.get(dcg_id)

evaluation, remaining_conditions = workflow.evaluate_trigger_conditions(
event_data, when_data_conditions
event_data, when_data_conditions, when_condition_group
)

if remaining_conditions:
Expand Down
14 changes: 10 additions & 4 deletions tests/sentry/workflow_engine/models/test_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,21 +31,27 @@ def test_queryset(self) -> None:
assert not Workflow.objects.filter(id=self.workflow.id).exists()

def test_evaluate_trigger_conditions__condition_new_event__True(self) -> None:
evaluation, _ = self.workflow.evaluate_trigger_conditions(self.event_data)
evaluation, _ = self.workflow.evaluate_trigger_conditions(
self.event_data, group=self.data_condition_group
)
assert evaluation.triggered is True

def test_evaluate_trigger_conditions__condition_new_event__False(self) -> None:
# Update event to have been seen before
self.group_event.group.times_seen = 5

evaluation, _ = self.workflow.evaluate_trigger_conditions(self.event_data)
evaluation, _ = self.workflow.evaluate_trigger_conditions(
self.event_data, group=self.data_condition_group
)
assert evaluation.triggered is False

def test_evaluate_trigger_conditions__no_conditions(self) -> None:
self.workflow.when_condition_group = None
self.workflow.save()

evaluation, _ = self.workflow.evaluate_trigger_conditions(self.event_data)
evaluation, _ = self.workflow.evaluate_trigger_conditions(
self.event_data, group=self.data_condition_group
)
assert evaluation.triggered is True

def test_evaluate_trigger_conditions__slow_condition(self) -> None:
Expand All @@ -57,7 +63,7 @@ def test_evaluate_trigger_conditions__slow_condition(self) -> None:
)
self.data_condition_group.conditions.add(slow_condition)
evaluation, remaining_conditions = self.workflow.evaluate_trigger_conditions(
self.event_data
self.event_data, group=self.data_condition_group
)

assert evaluation.triggered is True
Expand Down
Loading