Skip to content

Commit a23ada4

Browse files
snopokeclaude
andcommitted
Fix eager Celery task handling
Only close the session that the signal handlers opened; an eager task inside a caller's `Session()` was closing it out from under them. Also read per-call heartbeat options back out of the headers, which is where they stay when `before_task_publish` doesn't run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 96ec3a0 commit a23ada4

2 files changed

Lines changed: 77 additions & 8 deletions

File tree

‎taskbadger/celery.py‎

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@
2727
TB_TASK_ID = f"{KWARG_PREFIX}task_id"
2828
TB_HEARTBEAT_INTERVAL = f"{KWARG_PREFIX}heartbeat_interval"
2929
TB_STALE_TIMEOUT = f"{KWARG_PREFIX}stale_timeout"
30+
# Marks a request whose signal handlers opened the Task Badger session, so that
31+
# only they close it again.
32+
TB_OWNS_SESSION = f"{KWARG_PREFIX}owns_session"
3033

3134
log = logging.getLogger("taskbadger")
3235

@@ -268,15 +271,18 @@ def _maybe_create_task(signal_sender):
268271
if not manual_track and not auto_track:
269272
return
270273

271-
enter_session()
274+
enter_session(signal_sender)
272275

273276
delivery_info = getattr(signal_sender.request, "delivery_info", None) or {}
274277
queue = delivery_info.get("routing_key")
275278
external_id = signal_sender.request.id
276279
create_kwargs = {"status": StatusEnum.PENDING, "data": data, "queue": queue, "external_id": external_id}
280+
# `before_task_publish` never ran for these, so per-call options are still
281+
# sitting in the headers rather than resolved into the message.
282+
header_kwargs = headers.get(TB_KWARGS_ARG) or {}
277283
heartbeat_interval, stale_timeout = resolve_heartbeat_options(
278-
getattr(signal_sender, TB_HEARTBEAT_INTERVAL, None),
279-
getattr(signal_sender, TB_STALE_TIMEOUT, None),
284+
header_kwargs.get("heartbeat_interval", getattr(signal_sender, TB_HEARTBEAT_INTERVAL, None)),
285+
header_kwargs.get("stale_timeout", getattr(signal_sender, TB_STALE_TIMEOUT, None)),
280286
celery_system,
281287
)
282288
if stale_timeout is not None:
@@ -341,7 +347,7 @@ def _update_task(signal_sender, status, einfo=None):
341347
# ignore tasks that have already been set to a terminal state (probably in the task body)
342348
return
343349

344-
enter_session()
350+
enter_session(signal_sender)
345351

346352
data = None
347353
if einfo:
@@ -370,22 +376,32 @@ def _start_heartbeat(signal_sender):
370376
heartbeat.start(task_id, interval)
371377

372378

373-
def enter_session():
379+
def enter_session(signal_sender):
374380
if not Badger.is_configured():
375381
return
376382
session = Badger.current.session()
377-
if not session.client:
378-
session.__enter__()
383+
if session.client:
384+
# Already open, e.g. an eager task running inside the caller's
385+
# `taskbadger.Session()`. Not ours, so don't close it either.
386+
return
387+
session.__enter__()
388+
signal_sender.request.update({TB_OWNS_SESSION: True})
379389

380390

381391
def exit_session(signal_sender):
392+
request = signal_sender.request
382393
# not `request.headers`: eager tasks carry the ID on the request itself
383-
task_id = _get_taskbadger_task_id(signal_sender.request)
394+
# (the task id is checked first so that untracked tasks never touch `Badger`)
395+
task_id = _get_taskbadger_task_id(request)
384396
if not task_id or not Badger.is_configured():
385397
return
386398

387399
task_cache.unset(task_id)
388400

401+
if not request.get(TB_OWNS_SESSION):
402+
return
403+
request.update({TB_OWNS_SESSION: False})
404+
389405
session = Badger.current.session()
390406
if session.client:
391407
session.__exit__()

‎tests/test_celery_heartbeat.py‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
import pytest
1111

12+
import taskbadger
1213
from taskbadger import StatusEnum
1314
from taskbadger.celery import Task
1415
from taskbadger.mug import Badger, Settings
@@ -154,6 +155,58 @@ def add_eager(a, b):
154155
heartbeat.stop.assert_called_once_with(tb_task.id)
155156

156157

158+
@pytest.mark.usefixtures("_bind_settings_with_heartbeat")
159+
def test_heartbeat_for_eager_task_from_apply_async(celery_session_app, heartbeat):
160+
"""``before_task_publish`` doesn't run for eager tasks, so the per-call
161+
options have to be read back out of the headers."""
162+
163+
@celery_session_app.task(base=Task)
164+
def add_eager_options(a, b):
165+
return a + b
166+
167+
celery_session_app.conf.task_always_eager = True
168+
try:
169+
with (
170+
mock.patch("taskbadger.celery.create_task_safe") as create,
171+
mock.patch("taskbadger.celery.update_task_safe"),
172+
mock.patch("taskbadger.sdk.get_task"),
173+
):
174+
tb_task = task_for_test()
175+
create.return_value = tb_task
176+
result = add_eager_options.apply_async((2, 2), taskbadger_heartbeat_interval=3)
177+
assert result.get(timeout=10, propagate=True) == 4
178+
finally:
179+
celery_session_app.conf.task_always_eager = False
180+
181+
assert create.call_args.kwargs["stale_timeout"] == 6
182+
heartbeat.start.assert_called_once_with(tb_task.id, 3)
183+
184+
185+
@pytest.mark.usefixtures("_bind_settings_with_heartbeat")
186+
def test_eager_task_leaves_an_open_session_alone(celery_session_app, heartbeat):
187+
"""An eager task must not close a session opened by its caller."""
188+
189+
@celery_session_app.task(base=Task)
190+
def add_in_session(a, b):
191+
return a + b
192+
193+
celery_session_app.conf.task_always_eager = True
194+
try:
195+
with (
196+
mock.patch("taskbadger.celery.create_task_safe", return_value=task_for_test()),
197+
mock.patch("taskbadger.celery.update_task_safe", return_value=task_for_test()),
198+
mock.patch("taskbadger.sdk.get_task"),
199+
mock.patch("taskbadger.mug.AuthenticatedClient"),
200+
):
201+
with taskbadger.Session():
202+
session = Badger.current.session()
203+
assert add_in_session.delay(2, 2).get(timeout=10, propagate=True) == 4
204+
assert session.client is not None, "the caller's session was closed by the task"
205+
assert session.client is None
206+
finally:
207+
celery_session_app.conf.task_always_eager = False
208+
209+
157210
@pytest.mark.usefixtures("_bind_settings")
158211
def test_no_heartbeat_by_default(celery_session_app, celery_session_worker, heartbeat):
159212
@celery_session_app.task(base=Task)

0 commit comments

Comments
 (0)