Skip to content

Commit 98ab3bc

Browse files
markstoryandrewshie-sentry
authored andcommitted
fix(celery) Use model_name in process_incr (#87863)
Start using the model name instead of the model class when scheduling buffer process tasks. By replacing model with model_name we remove the last pickle parameters from celery. Support for `model` parameter has been preserved as when these changes ship, we'll have tasks in queues using `model`. Refs #81913
1 parent ef1fd34 commit 98ab3bc

File tree

6 files changed

+34
-13
lines changed

6 files changed

+34
-13
lines changed

src/sentry/buffer/base.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ def incr(
123123
"""
124124
process_incr.apply_async(
125125
kwargs={
126-
"model": model,
126+
"model_name": f"{model._meta.app_label}.{model._meta.model_name}",
127127
"columns": columns,
128128
"filters": filters,
129129
"extra": extra,

src/sentry/celery.py

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,8 @@
1010

1111
from sentry.utils import metrics
1212

13-
LEGACY_PICKLE_TASKS = frozenset(
14-
[
15-
# basic tasks that must be passed models still
16-
"sentry.tasks.process_buffer.process_incr",
17-
]
18-
)
13+
# XXX: Pickle parameters are not allowed going forward
14+
LEGACY_PICKLE_TASKS: frozenset[str] = frozenset([])
1915

2016

2117
def holds_bad_pickle_object(value, memo=None):
@@ -44,7 +40,8 @@ def holds_bad_pickle_object(value, memo=None):
4440
"django database models are large and likely to be stale when your task is run. "
4541
"Instead pass primary key values to the task and load records from the database within your task.",
4642
)
47-
if type(value).__module__.startswith(("sentry.", "getsentry.")):
43+
app_module = type(value).__module__
44+
if app_module.startswith(("sentry.", "getsentry.")):
4845
return value, "do not pickle custom classes"
4946

5047
return None

src/sentry/event_manager.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1800,6 +1800,10 @@ def _process_existing_aggregate(
18001800
**incoming_metadata,
18011801
"title": _get_updated_group_title(existing_metadata, incoming_metadata),
18021802
}
1803+
initial_priority = updated_group_values["data"]["metadata"].get("initial_priority")
1804+
if initial_priority is not None:
1805+
# cast to an int, as we don't want to pickle enums into task args.
1806+
updated_group_values["data"]["metadata"]["initial_priority"] = int(initial_priority)
18031807

18041808
# We pass `times_seen` separately from all of the other columns so that `buffer_inr` knows to
18051809
# increment rather than overwrite the existing value

src/sentry/tasks/process_buffer.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,21 @@ def process_incr(
7070
"""
7171
from sentry import buffer
7272

73-
if not model and model_name:
73+
if model:
74+
# Using model parameter in the celery task is deprecated
75+
# as we're trying to eliminate parameters that require pickle
76+
logger.info(
77+
"process_incr.model_kwarg",
78+
extra={
79+
"model": model,
80+
"columns": columns,
81+
"filters": filters,
82+
"extra": extra,
83+
"signal_only": signal_only,
84+
},
85+
)
86+
87+
if model_name:
7488
assert "." in model_name, "model_name must be in form `sentry.Group`"
7589
model = apps.get_model(model_name)
7690

tests/sentry/buffer/test_base.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,17 @@ def setUp(self):
2222

2323
@mock.patch("sentry.buffer.base.process_incr")
2424
def test_incr_delays_task(self, process_incr):
25-
model = mock.Mock()
25+
model = Group
2626
columns = {"times_seen": 1}
2727
filters: dict[str, BufferField] = {"id": 1}
2828
self.buf.incr(model, columns, filters)
29-
kwargs = dict(model=model, columns=columns, filters=filters, extra=None, signal_only=None)
29+
kwargs = dict(
30+
model_name="sentry.group",
31+
columns=columns,
32+
filters=filters,
33+
extra=None,
34+
signal_only=None,
35+
)
3036
process_incr.apply_async.assert_called_once_with(kwargs=kwargs, headers=mock.ANY)
3137

3238
def test_process_saves_data(self):

tests/sentry/tasks/test_process_buffer.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,10 @@ class ProcessIncrTest(TestCase):
1919
def test_constraints_model_name(self, process):
2020
with pytest.raises(AssertionError) as err:
2121
process_incr(model_name="group", columns={"times_seen": 1}, filters={"pk": 1})
22-
assert "model_name" in str(err)
22+
assert "model_name must be in form" in str(err)
2323

2424
@mock.patch("sentry.buffer.backend.process")
25-
def test_calls_process(self, process):
25+
def test_calls_process_with_model(self, process):
2626
columns = {"times_seen": 1}
2727
filters = {"pk": 1}
2828
process_incr(model=Group, columns=columns, filters=filters)

0 commit comments

Comments
 (0)