-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
feat(tasks) Add a simpler control for taskworker tasks #97000
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
Conversation
While we still support celery and taskworkers (for self-hosted) we can simplify the options and option checking logic. Once this option is deployed, I'll enable it for all regions and local development and then for self-hosted when documentation is published. Refs getsentry/taskbroker#453
@@ -497,6 +498,7 @@ def test_project_config_invalidations_delayed( | |||
assert schedule_inner.call_count == 2 | |||
|
|||
|
|||
@override_options({"taskworker.enabled": True}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ensures that a BurstTaskRunner
test will continue to behave correctly when taskworker tasks are the default.
src/sentry/taskworker/task.py
Outdated
def _signal_send(self, task: Task, args: Any, kwargs: Any) -> None: | ||
""" | ||
This method is a stub that sentry.testutils.task_runner.BurstRunner or other testing | ||
hooks can monkeypatch to capture tasks that are being produced. | ||
""" | ||
pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is necessary as there are a few wrappers around apply_async
for silo limiting and taskworker/celery rollout. When the taskworker/celery wrapper is gone we may be able to remove this.
This method is a stub that sentry.testutils.task_runner.BurstRunner or other testing | ||
hooks can monkeypatch to capture tasks that are being produced. | ||
""" | ||
pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Task Execution Duplication in Test Mode
The newly introduced _signal_send
method in TaskworkerTask.apply_async
causes tasks to execute twice in test environments. This occurs because the hook is called before the task's normal execution, leading to immediate execution when TASKWORKER_ALWAYS_EAGER
is true, and subsequent re-execution by test runners that capture tasks via the hook. Additionally, the _signal_send
stub's type hints (args: Any, kwargs: Any
) are inconsistent with the more specific types expected by test runner patches.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #97000 +/- ##
==========================================
- Coverage 80.66% 80.65% -0.02%
==========================================
Files 8530 8529 -1
Lines 375037 374800 -237
Branches 24372 24372
==========================================
- Hits 302524 302285 -239
- Misses 72136 72138 +2
Partials 377 377 |
While we still support celery and taskworkers (for self-hosted) we can simplify the options and option checking logic. Once this option is deployed, I'll enable it for all regions and local development and then for self-hosted when documentation is published. Refs getsentry/taskbroker#453
While we still support celery and taskworkers (for self-hosted) we can simplify the options and option checking logic.
Once this option is deployed, I'll enable it for all regions and local development and then for self-hosted when documentation is published.
Refs getsentry/taskbroker#453