Repository navigation
Conversation
919599a to
8a96faf
Compare
|
The only failing job is test-confluent-real. It passed on the previous head, while the current head changes only RabbitMQ code and tests. This attempt passed 274 of 277 Confluent tests; the three failures are existing docs tests that exhausted their flaky reruns after consumer-group LeaveGroupRequest timeouts. The RabbitMQ real tests and every other completed job passed. GitHub does not allow me to rerun the upstream job from the fork because that action requires repository admin permission. Could a maintainer rerun the failed job? |
8a96faf to
99d490c
Compare
|
Addressed the review in
The updated CI is fully green. @IvanKirpichnikov, could you take another look? |
| "durable": queue.durable, | ||
| "exclusive": queue.exclusive, | ||
| "auto_delete": queue.auto_delete, | ||
| "arguments": deepcopy(queue.arguments or {}), |
| "type": exchange.type, | ||
| "durable": exchange.durable, | ||
| "auto_delete": exchange.auto_delete, | ||
| "arguments": deepcopy(exchange.arguments or {}), |
| @pytest.mark.rabbit() | ||
| @pytest.mark.asyncio() | ||
| async def test_disconnect_clears_declaration_settings( | ||
| async_mock: AsyncMock, | ||
| queue: str, | ||
| ) -> None: | ||
| declarer = RabbitDeclarerImpl(FakeChannelManager(async_mock)) | ||
| await declarer.declare_queue(RabbitQueue(queue)) | ||
|
|
||
| with pytest.raises(SetupError): | ||
| await declarer.declare_queue(RabbitQueue(queue, durable=False)) | ||
|
|
||
| declarer.disconnect() | ||
| await declarer.declare_queue(RabbitQueue(queue, durable=False)) | ||
|
|
||
| assert async_mock.declare_queue.await_count == 2 |
There was a problem hiding this comment.
What is the purpose of the test?
| arguments["custom"].append("second") | ||
|
|
||
| with pytest.raises(SetupError, match=r"RabbitExchange .*arguments"): | ||
| await declarer.declare_exchange(schema) |
There was a problem hiding this comment.
It’s better to create a new RabbitExchange object instead of mutating schema.
| schema = RabbitQueue(queue, arguments=arguments) | ||
| await declarer.declare_queue(schema) | ||
|
|
||
| arguments["custom"].append("second") | ||
|
|
||
| with pytest.raises(SetupError, match=r"RabbitQueue .*arguments"): | ||
| await declarer.declare_queue(schema) |
There was a problem hiding this comment.
It’s better to create a new RabbitQueue object instead of mutating schema.
99d490c to
8298145
Compare
|
Thanks for the review. I removed both deep copies and changed the nested-argument tests to use separate |
This branch has not been deployed
2 participants
Description
Cache RabbitMQ queues and exchanges by name. Reusing an active declaration with different broker settings now raises
SetupErrorbefore another broker call; passive lookups reuse a cached object without treating their placeholder settings as a declaration. An active declaration made after a passive lookup replaces that cache entry. Binding-only settings are not part of declaration identity.Using the object name as the cache key also allows nested declaration arguments that cannot be hashed. The tests use separate
RabbitQueueandRabbitExchangeinstances when checking changed settings.Addresses #2034. The issue's original
0.6.0branch is no longer available, so this PR targetsmain.Validation
faststreamand the changed declaration test file; the full run also passed when excluding an unrelated Windows-onlySIGKILLtest file