fix(logs): import emit_signal from signals facade#60955
Merged
Conversation
The signals facade refactor (#60848) moved api.py to facade/api.py and updated every known caller, but it predated the alert signal emitter added in #60637, so that caller still imported the old products.signals.backend.api path. The old module no longer exists on master, so any process that loads the logs temporal activities (e.g. the session-replay temporal worker) fails at import time with ModuleNotFoundError and crash-loops. Point the import at the new facade path.
Contributor
|
Reviews (1): Last reviewed commit: "fix(logs): import emit_signal from signa..." | Re-trigger Greptile |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Every Temporal worker crash-loops at import time on prod-eu and prod-us with:
The signals facade refactor (#60848) moved
products/signals/backend/api.pytoproducts/signals/backend/facade/api.pyand updated every caller it knew about. It branched before #60637 addedproducts/logs/backend/alert_signal_emitter.py, so that one caller was missed and still imported the oldproducts.signals.backend.apipath.The old module no longer exists on master.
posthog/temporal/common/worker.pyimportsproducts.logs.backend.temporalunconditionally at module level, and that package pulls in the alert signal emitter — so any worker started viastart_temporal_worker(session replay, video export, batch exports, data modeling, tasks, …) fails to import and crash-loops. Not session-replay specific.Changes
One-line import fix in
alert_signal_emitter.py, pointing at the new facade path — matching what #60848 did for every other caller, and routing through the signals product's public facade as the isolation boundary expects.How did you test this code?
Agent-authored. No manual testing. Automated checks run:
django.setup()→import products.logs.backend.temporal→from products.logs.backend.alert_signal_emitter import emit_signal) — resolves cleanly,emit_signal.__module__is nowproducts.signals.backend.facade.api.ty checkpassed.git grepthis was the only stale importer of the old path on master (the remaining hit is a frontend doc comment).Automatic notifications
🤖 Agent context
Authored by Claude Code (Opus 4.8) at the user's direction, triggered by a worker crash-loop alert. The agent read the alert traceback, traced the import chain, and used
githistory to identify that #60848 (the facade move) predated the #60637 caller — a stale-merge miss rather than a logic bug. Verified the fix by reproducing the worker import chain under Django rather than relying on static reasoning, since the failure was import-time. Chose the facade path over re-adding the old module to stay consistent with the isolation boundary.