Skip to content

Conversation

@cathteng
Copy link
Member

Title

Should merge #102918 first. Also removes need for get_detectors_by_groupevents_bulk!

@cathteng cathteng requested review from a team as code owners November 10, 2025 18:56
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 10, 2025
@codecov
Copy link

codecov bot commented Nov 10, 2025

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
29900 1 29899 244
View the top 1 failed test(s) by shortest run time
tests.sentry.workflow_engine.processors.test_workflow_fire_history.TestWorkflowFireHistory::test_create_workflow_fire_histories_only_canonical
Stack Traces | 3.74s run time
#x1B[1m#x1B[.../workflow_engine/processors/test_workflow_fire_history.py#x1B[0m:43: in test_create_workflow_fire_histories_only_canonical
    result = create_workflow_fire_histories(
#x1B[1m#x1B[.../workflow_engine/utils/scopedstats.py#x1B[0m:166: in wrapper
    return f(*args, **kwargs)
#x1B[1m#x1B[31mE   TypeError: create_workflow_fire_histories() got multiple values for argument 'is_single_processing'#x1B[0m

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

total_actions += len(filtered_actions)

fire_actions(filtered_actions, detector, workflow_event_data)
fire_actions(filtered_actions, workflow_event_data)
Copy link
Contributor

Choose a reason for hiding this comment

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

Bug: Invalid Events Trigger Failing Actions

Removing the detector existence check allows workflow fire histories to be created and actions to be triggered for events without detectors. Previously, events missing detectors were skipped with a warning. Now, trigger_action tasks will be queued and fail when get_detector_by_event raises Detector.DoesNotExist, causing unnecessary task failures and retries instead of gracefully handling missing detectors upfront.

Fix in Cursor Fix in Web

Copy link
Member Author

Choose a reason for hiding this comment

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

This is fine

@cathteng cathteng force-pushed the cathy/aci/remove-detector-wfh-actions branch from e5270d6 to 62d99bf Compare November 20, 2025 17:07
Copy link
Member

@kcons kcons left a comment

Choose a reason for hiding this comment

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

End of an era.

@cathteng cathteng merged commit 2140ba3 into master Nov 24, 2025
65 of 67 checks passed
@cathteng cathteng deleted the cathy/aci/remove-detector-wfh-actions branch November 24, 2025 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants