eschutho commented on PR #45478: URL: https://github.com/apache/superset/pull/45478#issuecomment-6481572437
@rusackas thanks for the careful review. Both blockers are fixed in 6ed6df8866. **1. Leaked database session.** When the configured context wraps the whole listener thread, the old per-poll cleanup (which happened as each poll's temporary app context closed) no longer runs. A failed poll could leave the database session stuck in a failed transaction, so every later poll would fail too and abort signals would never arrive. The abort check now rolls back and discards the session whenever a poll raises, then re-raises so the listener's existing retry/backoff still applies. Added tests for both a failing and a succeeding poll. **2. Crash outside an app context.** The factory setting was read on the calling thread unconditionally, so any caller of `listen_for_signal` without an active app context hit a `RuntimeError`, even with no factory configured. It now checks for an active app context first and falls back to the previous no-op behavior if there isn't one. Added a test covering that case. Still open and intentionally not in this change: @geido's point that a transient failure while *entering* the factory's context ends the listener thread instead of retrying. Could you take another look when you have a chance? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
