-
Notifications
You must be signed in to change notification settings - Fork 3.1k
[SB] fix to capture properties when using batches #42598
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
/azp run python - servicebus - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
sdk/servicebus/azure-servicebus/azure/servicebus/_common/message.py
Outdated
Show resolved
Hide resolved
sdk/servicebus/azure-servicebus/azure/servicebus/_common/message.py
Outdated
Show resolved
Hide resolved
sdk/servicebus/azure-servicebus/azure/servicebus/_common/message.py
Outdated
Show resolved
Hide resolved
/azp run python - servicebus - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes an issue where batched messages with session IDs and partition keys could not be sent to Service Bus queues. The problem occurred because the batch envelope wasn't properly configured with the necessary properties and annotations from the individual messages.
Key changes:
- Added utility functions to set message properties and annotations on AMQP messages
- Modified the batch message creation to populate the batch envelope with session ID, message ID, and partition key from the first message
- Followed the .NET SDK pattern for handling batched message properties
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
sdk/servicebus/azure-servicebus/azure/servicebus/_pyamqp/utils.py |
Added utility functions set_message_properties and set_message_annotations for configuring AMQP message properties |
sdk/servicebus/azure-servicebus/azure/servicebus/_common/message.py |
Modified batch message logic to populate envelope properties from the first message when creating batches |
sdk/servicebus/azure-servicebus/azure/servicebus/_common/message.py
Outdated
Show resolved
Hide resolved
sdk/servicebus/azure-servicebus/azure/servicebus/_common/message.py
Outdated
Show resolved
Hide resolved
/azp run python - servicebus - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run python - servicebus - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run python - servicebus - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run python - servicebus - tests |
/azp run python - eventhub - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
1 similar comment
Azure Pipelines successfully started running 1 pipeline(s). |
sdk/servicebus/azure-servicebus/tests/async_tests/test_sessions_async.py
Show resolved
Hide resolved
/azp run python - servicebus - tests |
/azp run python - eventhub - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
1 similar comment
Azure Pipelines successfully started running 1 pipeline(s). |
/check-enforcer override |
This PR addresses an issue where batched messages that were sent to a session & partition enabled queue could not be sent. The error was happening because we not setting the application properties and session id on the batch for the messages. Singular messages would work just fine
We followed the pattern in .NET AMQP here https://github.com/Azure/azure-sdk-for-net/blob/a1af2cbe8117b706ce2c99ac04c084eea1e24334/sdk/servicebus/Azure.Messaging.ServiceBus/src/Amqp/AmqpMessageConverter.cs#L102
code to repro the issue