feat(workflow_engine): Setup the Task for process_workflow_updates #93553
Conversation
github-actions
bot
added
the
Scope: Backend
❌ 3 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
saponifi3d
force-pushed
the
jcallender/aci/workflow-status-update-handler
branch
from
5001848 to
8b6fffc
Compare
## Description Starting implementation for: https://www.notion.so/sentry/Workflow-Status-Changes-1fa8b10e4b5d80a48acddb95d160da1f?source=copy_link#1fa8b10e4b5d80e6bb1aef39cab2c6dc This will add a registry of handlers when an activity is created, we'll use this in the [workflow engine to kick-off a task](#93553) to execute `process_workflows`. This also adds a way to pass the detector_id through the StatusMessageData all the way to a hook that we can invoke and read directly from the StatusMessageData later. We are also passed the `activity` so `workflow_engine` can trigger `.send_notification` (if evaluated to do so).
- Create a task for handling activity updates
saponifi3d
force-pushed
the
jcallender/aci/workflow-status-update-handler
branch
from
8814284 to
62fc6a1
Compare
| max_retries=5, | ||
| soft_time_limit=50, | ||
| time_limit=60, | ||
| silo_mode=SiloMode.REGION, | ||
| taskworker_config=config.TaskworkerConfig( | ||
| namespace=namespaces.workflow_engine_tasks, | ||
| processing_deadline_duration=60, | ||
| retry=retry.Retry( | ||
| times=3, | ||
| delay=5, |
There was a problem hiding this comment.
Anyone have any thoughts on what these values for retry / limits should be? just using what we have been.
There was a problem hiding this comment.
There isn't any right/wrong answer here. However, if you are going to define Retry you should also define on which exceptions that retrys should be performed. Without that retries will only be automatically performed for TimeoutError or an explicit RetryError.
There was a problem hiding this comment.
👍 thanks for the additional info -- seems like a number we can fudge around with in the future then, and we can use the same configuration as we are with issue alert processing (that's where i pulled these initial numbers from)
There was a problem hiding this comment.
when would we not want to go with acks_late=True in cases where we'd rather double process than drop one?
| max_retries=5, | ||
| soft_time_limit=50, | ||
| time_limit=60, | ||
| silo_mode=SiloMode.REGION, | ||
| taskworker_config=config.TaskworkerConfig( | ||
| namespace=namespaces.workflow_engine_tasks, | ||
| processing_deadline_duration=60, | ||
| retry=retry.Retry( | ||
| times=3, | ||
| delay=5, |
There was a problem hiding this comment.
There isn't any right/wrong answer here. However, if you are going to define Retry you should also define on which exceptions that retrys should be performed. Without that retries will only be automatically performed for TimeoutError or an explicit RetryError.
|
|
||
| The task will get the Activity from the database, create a WorkflowEventData object, | ||
| and then process the data in `process_workflows`. | ||
| """ |
There was a problem hiding this comment.
Any ideas on what kind of throughput tasks in the workflow_tasks namespace will have?
There was a problem hiding this comment.
Hmm - not sure about throughput, i think that it will vary pretty heavily depending on configurations / products. for example, a metric issue with anomaly detection will be more computing time than evaluating if an event is new.
In the end, the throughput would be similar to issue alerts / metric alerts / crons -- all three of those product verticals will be executing here, but they're all fairly different rates.
Description
In the code I mention there being a follow-up PR: #93580 is the initial PR to support the
Activity/Groupmodels.