fix(pidbox): default to exclusive queues for RabbitMQ 4.3.0 compatibility#2530
fix(pidbox): default to exclusive queues for RabbitMQ 4.3.0 compatibility#2530Nusnus wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2530 +/- ##
=======================================
Coverage 82.49% 82.49%
=======================================
Files 79 79
Lines 10190 10190
Branches 1170 1170
=======================================
Hits 8406 8406
Misses 1582 1582
Partials 202 202 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Pull request overview
Updates Kombu’s pidbox mailbox queue defaults to improve compatibility with RabbitMQ 4.3.0 behavior around transient, non-exclusive queues.
Changes:
- Change
Mailboxdefaultqueue_exclusivefromFalsetoTrue.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| queue_ttl=None, queue_expires=None, | ||
| queue_durable=False, queue_exclusive=False, | ||
| queue_durable=False, queue_exclusive=True, | ||
| reply_queue_ttl=None, reply_queue_expires=10.0): |
There was a problem hiding this comment.
Changing the default queue_exclusive to True makes Mailbox(..., queue_durable=True) raise ValueError unless callers also pass queue_exclusive=False, which is a backwards-incompatible behavior change. Consider using a sentinel/None default for queue_exclusive and deriving the effective value from queue_durable (e.g., default to exclusive only when queue_durable is false), so existing durable-queue callers don’t break while still fixing the transient non-exclusive RabbitMQ case.
| queue_durable=False, queue_exclusive=True, | ||
| reply_queue_ttl=None, reply_queue_expires=10.0): |
There was a problem hiding this comment.
With queue_exclusive=True by default, starting a second node with the same hostname will likely fail during queue_declare (exclusive queue in use) before Queue.on_declared runs, so W_PIDBOX_IN_USE won’t be surfaced anymore. Consider catching the exclusive-queue declare failure and re-raising an InconsistencyError (or emitting the existing warning) with the actionable W_PIDBOX_IN_USE guidance, so users get a consistent, helpful message.
| queue_durable=False, queue_exclusive=True, | ||
| reply_queue_ttl=None, reply_queue_expires=10.0): |
There was a problem hiding this comment.
This changes the default queue flags (and may change runtime behavior/validation), but there’s no unit test asserting the new default behavior (e.g., that Mailbox('x') now creates exclusive, auto-delete pidbox + reply queues, and that Mailbox(..., queue_durable=True) still behaves as intended). Adding a focused test will prevent regressions across transports and clarify the intended defaults.
|
Attempted fix instead: #2531 |
No description provided.