Repository navigation
fix(sqs): scope queue cache, noack set and predefined clients to Connection - #2676
Merged
auvipy merged 5 commits intoSep 27, 2026
Merged
Conversation
…ection Channel._queue_cache, _noack_queues, _predefined_queue_clients and _predefined_queue_async_clients were class attributes, so every SQS Channel in the process shared them regardless of which Connection it belonged to. Two Connections to different regions or accounts that use the same queue name resolve to whichever queue URL was cached first, a Connection with different credentials reuses another Connection's boto3 client for a predefined queue, and a no_ack consumer on one Connection makes consumers of the same queue name on every other Connection delete messages on receipt. Move them onto the Transport, one per Connection, and expose them on Channel through properties, the same approach celery#2543 took for Azure Service Bus. Channels of one Connection still share them.
Member
auvipy
requested changes
Sep 26, 2026
auvipy
left a comment
auvipy
left a comment
Member
There was a problem hiding this comment.
can we also add integration tests for this aswell?
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2676 +/- ##
==========================================
+ Coverage 83.95% 83.97% +0.02%
==========================================
Files 80 80
Lines 11217 11231 +14
Branches 1345 1345
==========================================
+ Hits 9417 9431 +14
Misses 1583 1583
Partials 217 217 ☔ View full report in Codecov by Harness. |
Two Connections whose predefined_queues map the same queue name to different SQS queues, run against GoAws. Publishing through the first Connection after the second has opened a channel must still land in the first Connection's own queue. Fails on main, where the second channel overwrites the shared cache entry.
Contributor
Author
|
Added an integration test in t/integration/test_sqs.py that runs against GoAws. It creates two real queues and two Connections whose predefined_queues map |
Member
Member
|
tried to fix a newly emerged merge conflict due to a new merge in main branch |
auvipy
reviewed
Sep 27, 2026
auvipy
approved these changes
Sep 27, 2026
This was referenced Sep 27, 2026
auvipy
added a commit
that referenced
this pull request
Sep 30, 2026
Channel._queue_name_cache was a class attribute, so every Azure Storage Queues channel in the process shared one dict of queue names, filled from each storage account's list_queues(). A Connection to a second storage account then trusts the first account's listing: _ensure_queue finds the name in the cache, skips create_queue, and the send fails with ResourceNotFoundError because the queue only exists in the first account. Move the cache onto the Transport, one per Connection, and expose it on Channel through a property, the same approach as #2543 and #2676. Co-authored-by: Asif Saif Uddin {"Auvi":"অভি"} <[email protected]>
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.
The SQS
Channeldeclares_queue_cache,_noack_queues,_predefined_queue_clientsand_predefined_queue_async_clientsas class attributes, so every SQS channel in the process shares one dict/set per name regardless of whichConnectionit belongs to. This is the same problem #2543 fixed for Azure Service Bus.What that breaks on
main:ordersresolve to whichever URL was cached first, so one of them sends to and polls the other's queue.predefined_queues) but cached by queue name only, so a second Connection with different credentials gets the first one's client.no_ackleaks across Connections._noack_queuesdecides whether a received message is deleted immediately. Ano_ack=Trueconsumer on one Connection makes consumers of the same queue name on every other Connection delete messages on receipt, so a worker crash there loses the message instead of it being redelivered.The fix moves the four containers onto
Transport(one per Connection) and exposes them onChannelthrough properties, following #2543. Channels of the same Connection still share them, so caching behaves the same within a Connection.Tests: added
test_SQS_per_connection_state, which fails onmain(a channel on a second Connection sees the first one's cached URL) and passes with the change, plus a check that channels of one Connection still share.setup_methodno longer needs to clearSQS.Channel._queue_cachebetween tests, and three tests that reassigned the client maps now reset them in place. Fullt/unitrun: 1536 passed with the change, 1534 without (the two new tests), same two pre-existing Azure-deps collection errors either way. pre-commit hooks pass on both files.