fix(servicebus): Read max-message-batch-size vendor property for batch sizing#46197
Open
EldertGrootenboer wants to merge 6 commits intoAzure:mainfrom
Open
fix(servicebus): Read max-message-batch-size vendor property for batch sizing#46197EldertGrootenboer wants to merge 6 commits intoAzure:mainfrom
EldertGrootenboer wants to merge 6 commits intoAzure:mainfrom
Conversation
…lient (Azure#44999) - Pass **kwargs from __init__ to _build_pipeline() in both sync and async ServiceBusAdministrationClient so transport kwargs (connection_verify, transport, policies, ssl_context) reach the transport layer - Add unit tests verifying kwargs forwarding for both sync and async clients - Matches pattern from Azure#26015 fix for ServiceBusClient and azure-core base
…h sizing - Read com.microsoft:max-message-batch-size from AMQP sender link remote_properties to correctly limit batch size on Premium large-message entities where max-message-size can be up to 100 MB but the batch limit is 1 MB - Add get_remote_max_message_batch_size() to both pyamqp and uamqp transport layers (uamqp returns None as it doesn't expose link properties, falling back to existing tier-based inference) - Update both sync and async senders to prefer vendor property - Add comprehensive unit tests for transport method and sender batch size inference
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates Azure Service Bus sender batch sizing to prefer the service-reported AMQP vendor link property com.microsoft:max-message-batch-size, with a fallback to the existing tier-based inference when the property isn’t available (notably for the uAMQP transport). It also adjusts the management client to forward **kwargs into pipeline construction and adds regression tests.
Changes:
- Read
com.microsoft:max-message-batch-sizefrom pyamqp linkremote_propertiesand use it to set_max_batch_size_on_link(sync + async senders). - Add a uAMQP transport stub for the same API (returns
None, triggering fallback behavior). - Add unit tests for batch sizing and for forwarding management client
**kwargsinto_build_pipeline.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/servicebus/azure-servicebus/azure/servicebus/_transport/_pyamqp_transport.py | Adds get_remote_max_message_batch_size() reading from pyamqp link remote_properties. |
| sdk/servicebus/azure-servicebus/azure/servicebus/_transport/_uamqp_transport.py | Adds get_remote_max_message_batch_size() stub returning None for uAMQP. |
| sdk/servicebus/azure-servicebus/azure/servicebus/_servicebus_sender.py | Uses vendor batch-size property when available, otherwise falls back to tier inference. |
| sdk/servicebus/azure-servicebus/azure/servicebus/aio/_servicebus_sender_async.py | Async sender mirrors the same vendor-property-first batch sizing behavior. |
| sdk/servicebus/azure-servicebus/azure/servicebus/management/_management_client.py | Forwards **kwargs into _build_pipeline(**kwargs). |
| sdk/servicebus/azure-servicebus/azure/servicebus/aio/management/_management_client_async.py | Async management client mirrors _build_pipeline(**kwargs) forwarding. |
| sdk/servicebus/azure-servicebus/tests/test_batch_sizing.py | Adds unit tests covering vendor property presence/absence and tier fallback logic. |
| sdk/servicebus/azure-servicebus/tests/test_mgmt_client_kwargs.py | Adds regression tests ensuring management client **kwargs reach _build_pipeline (sync + async). |
sdk/servicebus/azure-servicebus/azure/servicebus/_transport/_pyamqp_transport.py
Outdated
Show resolved
Hide resolved
sdk/servicebus/azure-servicebus/azure/servicebus/management/_management_client.py
Show resolved
Hide resolved
- Check both bytes and str forms of vendor property key since pyamqp decodes AMQP symbols as bytes (b'com.microsoft:max-message-batch-size') - Update tests to use bytes keys matching production behavior - Add test for str key fallback - Remove unused imports, fix docstring
sdk/servicebus/azure-servicebus/azure/servicebus/_transport/_uamqp_transport.py
Show resolved
Hide resolved
sdk/servicebus/azure-servicebus/azure/servicebus/aio/_servicebus_sender_async.py
Show resolved
Hide resolved
sdk/servicebus/azure-servicebus/azure/servicebus/aio/management/_management_client_async.py
Show resolved
Hide resolved
- Add get_remote_max_message_batch_size to AmqpTransport base class for type safety (mypy/IDE) - Use MAX_MESSAGE_LENGTH_BYTES constant in tests instead of hardcoded 256*1024
sdk/servicebus/azure-servicebus/azure/servicebus/_transport/_uamqp_transport.py
Outdated
Show resolved
Hide resolved
sdk/servicebus/azure-servicebus/azure/servicebus/aio/_servicebus_sender_async.py
Show resolved
Hide resolved
- Add pylint disable=unused-argument on uamqp stub method - Add async sender batch size inference tests for parity with sync tests
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.
Problem
On Premium large-message Service Bus entities, the AMQP link's
max-message-sizecan be up to 100 MB, but the broker enforces a 1 MB limit for batch sends. The Python SDK infers batch size from the tier by comparingmax-message-sizeagainst known thresholds — this works today but is fragile and doesn't use the actual service-reported value.Related: azure-service-bus#708
Fix
Read the
com.microsoft:max-message-batch-sizevendor property from the AMQP sender linkremote_properties, which correctly reports 256 KB (Standard) / 1 MB (Premium) independent of entity-levelmax-message-size. Fall back to the existing tier-based inference when the vendor property is absent (including the uAMQP transport which doesn't expose link properties).Changes
_pyamqp_transport.py: Addget_remote_max_message_batch_size()reading fromhandler._link.remote_properties_uamqp_transport.py: Add stubget_remote_max_message_batch_size()returningNone(uAMQP transport doesn't expose link properties; falls back to tier-based inference)_servicebus_sender.py+_servicebus_sender_async.py: Prefer vendor property, fall back to tier-based inference for both sync and async senderstest_batch_sizing.py: 12 new unit tests: transport method (8 tests: present, absent, wrong type, zero, negative, Standard, missing attr) + sender inference (4 tests: vendor override, Premium fallback, Standard fallback, Standard with vendor)Cross-SDK alignment