-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
feat(taskworker): Make ingest errors taskworker compatible #90340
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
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #90340 +/- ##
==========================================
- Coverage 87.78% 85.22% -2.56%
==========================================
Files 10276 10288 +12
Lines 582704 583825 +1121
Branches 22567 22658 +91
==========================================
- Hits 511543 497584 -13959
- Misses 70732 85695 +14963
- Partials 429 546 +117 |
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.
All of these tasks have untyped parameters (in some cases just **kwargs
). I think we should do some extra auditing to try and ensure that all the parameters these functions are called with can be serialized to JSON.
@evanh Just did an audit. It looks like |
src/sentry/conf/server.py
Outdated
@@ -1508,6 +1509,14 @@ def SOCIAL_AUTH_DEFAULT_USERNAME() -> str: | |||
}, | |||
} | |||
|
|||
# | |||
TASKWORKER_ENABLE_INGEST_NAMESPACES = False |
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 looks good to me but I wouldn't name this INGEST
, I think this is a concept we could use for other namespaces besides ingest that receive high traffic.
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.
Looks good, the extra settings will help prevent another option problem.
This work is required to migrate tasks from celery to the new taskbroker system. The sentry option will be used to control the rollout of these tasks. The full migration plan is describe in this document.