[ServiceBus] fix receive_messages receive and delete bug #31833
Conversation
|
API change check API changes are not detected in this pull request. |
|
/azp run python - servicebus - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| @@ -647,6 +647,8 @@ def enhanced_message_received( | |||
| if receiver._receive_context.is_set(): | |||
| receiver._handler._received_messages.put((frame, message)) | |||
| else: | |||
| if receiver._receive_mode == ServiceBusReceiveMode.RECEIVE_AND_DELETE: | |||
There was a problem hiding this comment.
just to double check have we tested iterator with receive/delete turned on to make sure we receive all the messages correctly
There was a problem hiding this comment.
yeah, I ran the receiveanddelete queue tests and they all passed! was there any test in particular that you were thinking?
There was a problem hiding this comment.
will this need another check on frame & message if prefetch is set to 0 ( in the other PR), because then it wont have any preloaded messages or frames that would be added to received messages ?
There was a problem hiding this comment.
when prefetch is 0, it should be using the default _message_received callback, not this one. So I don't think it should have issues?
There was a problem hiding this comment.
Maybe we could move this up into the original if-statement?
For example:
if receiver._receive_context.is_set() or receiver._receive_mode == 'RECEIVE_AND_DELETE':
I would follow this up with a descriptive inline comment or method docstring about what circumstances we want to keep messages around, vs releasing them. This entire enhanced_message_received method seems to be missing a lot of documentation about the reason for its existence. Without that context, the design is hard to explain.
If I recall correctly, messages received in PEEK_LOCK need to be released to prevent them accumulating deliver-attempt counts when they expire without settlement - is that right?
If so, we should add a comment explaining how RECEIVE_AND_DELETE fits into that pattern.
|
closing in favor of #31873 |
fixes: #31711
When prefetch is set to 1, (which it is by default + will need to be addressed in the related #31712 issue fix), the message_received callback on the AMQP Receive Client is set to the
_enhanced_message_received. When a message is received in receive and delete mode, this callback doesn't append received messages to the internal q before settling right away. When the user calls receive_messages again a little later, those messages are not in the internal q, so they are not returned to the user.This bug also existed in previously released uamqp-based versions, so adding the fix to the UamqpTransport as well.