Repository navigation
[Service Bus] Add batch delete and purge APIs - #48845
Eldert Grootenboer (EldertGrootenboer) wants to merge 45 commits into
Conversation
|
Azure Pipelines: Successfully started running 1 pipeline(s). 9 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
🟡 Changes recommended
Management-link setup remains outside the promised retry and timeout guarantees, and Premium-only samples run unconditionally against Standard resources.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds receiver-level batch delete and purge support to Azure Service Bus, including synchronous and asynchronous APIs.
Changes:
- Adds delete/purge APIs and result models.
- Implements AMQP request handling, validation, cutoffs, and timeout budgets.
- Adds samples, tests, changelog, and API artifacts.
File summaries
| File | Description |
|---|---|
tests/unittests/test_batch_delete.py |
Tests protocol, validation, sessions, timeouts, and purge loops. |
samples/sync_samples/sample_code_servicebus.py |
Adds synchronous usage examples. |
samples/async_samples/sample_code_servicebus_async.py |
Adds asynchronous usage examples. |
CHANGELOG.md |
Documents the new APIs. |
azure/servicebus/aio/_servicebus_receiver_async.py |
Implements asynchronous delete and purge. |
azure/servicebus/aio/_base_handler_async.py |
Adds timeout-aware asynchronous opening. |
azure/servicebus/_servicebus_receiver.py |
Implements synchronous delete and purge. |
azure/servicebus/_models.py |
Defines result objects. |
azure/servicebus/_common/utils.py |
Adds datetime-to-milliseconds conversion. |
azure/servicebus/_common/mgmt_handlers.py |
Parses and validates delete responses. |
azure/servicebus/_common/constants.py |
Adds management operation constants. |
azure/servicebus/_base_handler.py |
Adds timeout-aware synchronous opening. |
azure/servicebus/__init__.py |
Exports result types. |
api.metadata.yml |
Updates the API snapshot hash. |
api.md |
Records the new public API surface. |
Review details
- Files reviewed: 15/15 changed files
- Comments generated: 6
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
- Align management-link transport documentation - Resolve batch delete typing across sync and async clients
There was a problem hiding this comment.
🟡 Changes recommended
Management-link setup cannot reliably recover from attach failures, and uAMQP bypasses the promised setup retry.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
sdk/servicebus/azure-servicebus/azure/servicebus/_pyamqp/aio/_client_async.py:379
- The async vendored pyamqp change is not mirrored in Event Hubs (
sdk/eventhub/azure-eventhub/azure/eventhub/_pyamqp/aio/_client_async.py:345-392). The two vendored stacks are kept byte-identical; this introduces transport behavior drift and complicates future shared fixes. Apply the same async management-link refactor to the Event Hubs copy.
sdk/servicebus/azure-servicebus/azure/servicebus/_pyamqp/client.py:466 - This vendored pyamqp client change is not mirrored in the Event Hubs copy (
sdk/eventhub/azure-eventhub/azure/eventhub/_pyamqp/client.py:432-478). These stacks are maintained as byte-identical copies; leaving only Service Bus updated creates divergent timeout, cleanup, and management-link behavior that makes future shared fixes unsafe. Apply the equivalent change to the Event Hubs vendored client.
sdk/servicebus/azure-servicebus/azure/servicebus/_pyamqp/aio/_client_async.py:406
asyncio.CancelledErroris not anException, so cancellation during either readiness loop skips this cleanup and leaves the management link cached in an indeterminate state. CatchBaseExceptionfor this cleanup boundary and immediately re-raise, as the method already does.
except Exception:
- Files reviewed: 23/23 changed files
- Comments generated: 4
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Timeout handling, response validation, and Event Hubs parity issues remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
Previously missed (3) — in code that hasn't changed since the last review.
sdk/servicebus/azure-servicebus/azure/servicebus/_base_handler.py:617
- When pyAMQP management-link setup exhausts its budget,
open_mgmt_link()raises the transport's built-inTimeoutError. Unlike_mgmt_request_response()(lines 544-546), this setup path does not translate that error before_do_retryable_operation()calls_handle_exception(), so callers receive a genericServiceBusErrorinstead ofOperationTimeoutError. Translate the transport timeout here to preserve the public timeout contract.
sdk/servicebus/azure-servicebus/azure/servicebus/_common/mgmt_handlers.py:87 - Returning zero for a 204 treats a response with no
message-countas successful exhaustion. This contradicts the PR contract that missing counts fail and purge stops only when the service explicitly returns zero; a bodyless response can therefore terminate purge while eligible messages remain. Reject this response unless it contains a validated count, and update the test that currently asserts204 == 0.
sdk/servicebus/azure-servicebus/azure/servicebus/aio/_base_handler_async.py:464 - When async pyAMQP management-link setup exhausts its budget,
open_mgmt_link_async()raises the transport's built-inTimeoutError. Unlike_mgmt_request_response()(lines 391-393), this setup path does not translate that error before_do_retryable_operation()calls_handle_exception(), so callers receive a genericServiceBusErrorinstead ofOperationTimeoutError. Translate the transport timeout here to preserve the public timeout contract.
- Files reviewed: 23/23 changed files
- Comments generated: 2
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Response validation and sync/async timeout-budgeting issues remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
sdk/servicebus/azure-servicebus/azure/servicebus/_common/utils.py:83
- Add a second blank line between these top-level functions. The same module consistently leaves two blank lines between definitions (for example, lines 97–100), and Black will otherwise reformat this file.
return (normalized - EPOCH_UTC) // datetime.timedelta(milliseconds=1)
- Files reviewed: 23/23 changed files
- Comments generated: 3
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Async timeout handling can leak a coroutine, and the async session sample may wait indefinitely.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
sdk/servicebus/azure-servicebus/samples/async_samples/sample_code_servicebus_async.py:482
- This default invocation targets
SERVICEBUS_SESSION_ID, but the preceding async session example creates only the literal"<your session id>"(lines 333-340), while the test deployment supplies a different generated ID. On a clean namespace, entering this receiver waits indefinitely for a session this script never created. Keep this session-specific example opt-in or seed the configured session before opening it.
sdk/servicebus/azure-servicebus/azure/servicebus/_common/utils.py:83
- Add a second blank line after this new top-level helper. The surrounding module uses two blank lines between top-level functions (for example, lines 97–100), and Black will reformat this section otherwise.
return (normalized - EPOCH_UTC) // datetime.timedelta(milliseconds=1)
sdk/servicebus/azure-servicebus/azure/servicebus/_servicebus_receiver.py:860
- The PR promises support for subqueue receivers, but the new suite exercises only regular and session receivers; there is no
sub_queuecase. Because dead-letter/transfer-dead-letter receivers use a different entity management target, add sync and async coverage that creates those receiver variants and verifies the batch-delete request is sent to the subqueue target.
def delete_messages(
self,
message_count: int,
*,
before_enqueued_time: Optional[datetime.datetime] = None,
- Files reviewed: 23/23 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Setup timeout conversion, async session sample seeding, and formatting issues remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
Previously missed (3) — in code that hasn't changed since the last review.
sdk/servicebus/azure-servicebus/azure/servicebus/_common/utils.py:85
- Only one blank line separates this new top-level helper from
utc_from_timestamp, while this file uses two between top-level functions (for example, lines 97–100). This violates the Black/PEP 8 layout and will fail the formatting check; add the second blank line.
sdk/servicebus/azure-servicebus/azure/servicebus/_transport/_pyamqp_transport.py:1093 open_mgmt_link()raises the transport's rawTimeoutError, but this setup call runs directly inside_do_retryable_operation, not_mgmt_request_response(which normally converts transport timeouts)._handle_exceptionconsequently wraps it as a non-retryable genericServiceBusError, closes the handler, and aborts setup instead of surfacingOperationTimeoutError. Convert the transport timeout here before it reaches the retry helper.
sdk/servicebus/azure-servicebus/samples/async_samples/sample_code_servicebus_async.py:450- This executable sample opens
SERVICEBUS_SESSION_ID, but the precedingexample_session_ops_async()seeds only the distinct literal"<your session id>". On a clean sample namespace there is therefore no active message/session for this ID, so the exact-session receiver can wait instead of demonstrating purge. Seed this session inside the example (as the sync flow effectively does) so the snippet also works independently.
- Files reviewed: 23/23 changed files
- Comments generated: 1
- Review effort level: Balanced
- Wrap the discovery parameter dictionary to match CI-pinned Black output. - Preserve the validated deterministic authority-discovery assertions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9519ad81-9610-4092-b343-887e9cabdda0
- Disable environment proxy discovery only for the two mocked transports. - Exercise registration success and failure with and without a proxy. - Restore environment variables after each test without changing production policy. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9519ad81-9610-4092-b343-887e9cabdda0
There was a problem hiding this comment.
🔵 Needs a closer look
Destructive Service Bus operations and shared management-link changes need live-broker validation by service owners, while repository-wide CI provisioning changes require engineering-system owner confirmation of job results.
No open issues 🎉
Review effort: Balanced
This comment has been minimized.
This comment has been minimized.
Pranjal Patel (pranz1996)
left a comment
There was a problem hiding this comment.
Re-reviewed db72cbb, including the expanded management-link setup/cleanup changes. Both earlier findings are addressed: destructive dispatch uses the standard exception translation/cleanup path and respects try_timeout. I found no new blocking issues in the Service Bus batch-delete/purge implementation.
Before signing off on the full PR, two PR-level concerns remain:
- Scope: nine changed files are outside sdk/servicebus/, including shared CI/native-dependency setup and Core/Identity tests. Please split those repairs into focused PRs where practical, or obtain review from the relevant owners.
- CI: the main Python PR pipeline and CodeQL pass, but AutoRest and Cosmos checks remain red. AutoRest fails downloading a TypeSpec tarball with HTTP 404. The Windows Cosmos jobs fail test_health_check_success (counter 3 versus 2) and test_activity_id_logging_policy (activity-ID mismatch). These do not establish a batch-delete defect, but please clear the failures or document why they are unrelated and acceptable before merge.
Holding approval of the full expanded diff pending that clarification; no additional Service Bus correctness changes requested.
|
The two Service Bus findings are addressed, as confirmed in #48845 (review). The nine files outside Service Bus repair failures in the required Python PR pipeline: Python 3.15 bootstrap compatibility, proxy-independent Core/ARM/Identity tests, and native test-dependency provisioning using the existing authenticated feed setup. Changing Core tests expands dependent-package validation, which exposed the Identity and native-dependency failures. These changes do not alter production Core/Identity behavior or relax security policy. The required Python build 7056524 passed on db72cbb. The final main integration is now 6f853a7; its required build 7059471 is still pending. Engineering, Core, Identity and ARM owner reviews are already requested; their approval remains outstanding. The shared repairs have not been split out, and owner review is the remaining path for accepting that scope. The reported AutoRest 7056522 feed 404 and Cosmos 7056521 emulator assertions remain visible. Neither check is required by the effective branch policy, and this PR does not change generator inputs or Cosmos code. That explains why they do not block the automated required-check gate; it does not establish that the failures are acceptable for merge. No waiver or optional-check retry has been applied. Your approval hold and the relevant owners' decisions remain open. |
- Adopt the typing-extensions 4.16.0 pin merged in upstream PR 49915. - Resolve the only overlapping upstream change without importing unrelated packages. - Preserve the required Python 3.15 dispatcher fix and all SDK behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9519ad81-9610-4092-b343-887e9cabdda0
There was a problem hiding this comment.
🔵 Needs a closer look
Destructive Service Bus and shared AMQP lifecycle changes need broker-backed validation across sessions, subqueues, and both transports, while repository-wide CI changes also require pipeline validation and engineering-system owner review.
No open issues 🎉
Review effort: Balanced
This comment has been minimized.
This comment has been minimized.
- Install the test server declared by the generator without its unused workspace dependencies. - Preserve authenticated registry setup and both complete legacy suites. - Install PR Core packages last and fail immediately if setup fails. - Cover dependency selection and package installation order with focused owner tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9519ad81-9610-4092-b343-887e9cabdda0
- Attribute startup probes to their owning connection instead of a process-wide counter. - Preserve exactly two probes and the expected regional endpoints. - Wait for the owned startup thread and close the test client. - Cover foreign-client interference and rejection of duplicate target probes without network access. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9519ad81-9610-4092-b343-887e9cabdda0
- Apply the exact Black24.4.0 configuration reported by engineering CI. - Preserve all three passing dependency and installation-order regressions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9519ad81-9610-4092-b343-887e9cabdda0
This comment has been minimized.
This comment has been minimized.
- Register instance-owned cleanup before awaiting container creation. - Preserve the existing management deadline and cleanup behavior. - Cover successful setup and cancellation with isolated fixture regressions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9519ad81-9610-4092-b343-887e9cabdda0
- Record cache file lifecycle and subprocess completion metadata. - Compare count-only memory and persisted lookup evidence on failure. - Preserve concurrent writers, authentication and isolation assertions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9519ad81-9610-4092-b343-887e9cabdda0
- Adopt the exact upstream live-test template to remove its conflict. - Preserve native provisioning, feed and authentication ordering coverage. - Leave shared CI preparation and SDK runtime unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9519ad81-9610-4092-b343-887e9cabdda0
- Include the observed refresh-token KeyError in failure-only counts. - Re-raise the original error without changing authentication behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9519ad81-9610-4092-b343-887e9cabdda0
- Wrap the diagnostic response exactly as Black 24.4.0 requires. - Preserve all cache isolation assertions and diagnostic behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9519ad81-9610-4092-b343-887e9cabdda0
[Pilot] PR Pipeline Failure AnalysisWhat failedPipeline: One test failed in
Relevant pipeline outputRoot causeThis test asserts that Not caused by this PR. PR #48845 ( Recommended next steps
|
- Close only the fixture-owned client instead of evicting other live clients' shared cache state. - Reproduce the stale-release ordering that leaves clear_cache operating on an orphaned dictionary. - Preserve the original emulator cache-clearing and repopulation assertions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9519ad81-9610-4092-b343-887e9cabdda0
- Assert the second client's exact endpoint instead of counting unrelated background probes globally. - Inject an unrelated client's probe explicitly while retaining strict owned-client counts and duplicate rejection. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9519ad81-9610-4092-b343-887e9cabdda0
There was a problem hiding this comment.
🟡 Changes recommended
The live-test stage does not expose or forward the three Rust options, leaving toolchain setup and credential forwarding disabled for pipelines using that stage; a forwarding regression test is also needed.
1 open issue
Review effort: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Human review is needed to confirm destructive API deletion scope and shared management-link behavior for sessions, subscriptions, and subqueues, since supplied live tests cover only ordinary queues.
No open issues 🎉
1 resolved since last review
Review effort: Balanced

Description
Azure Service Bus exposes the
com.microsoft:batch-delete-messagesmanagement operation. This PR adds synchronous and asynchronous receiver-level batch-delete and purge APIs toazure-servicebusfor regular, session, and subqueue receivers.Both methods return result objects containing
deleted_message_count. Purge captures one enqueue-time cutoff and one operation-wide timeout, defaults to 500 messages per request, continues after every positive result, and stops only when the service returns zero. Callers can select another positive signed 32-bit request size; Service Bus enforces the current tier limit of 500 for Basic and Standard and 4,000 for Premium.Management-link setup is retryable, but a destructive request is dispatched only once. Missing, malformed, negative, or excessive response counts fail instead of being interpreted as successful exhaustion. Samples cover default purge, Premium sizing, sessions, enqueue-time thresholds, returned counts, and partial or unknown outcomes.
Companion implementations: JavaScript, .NET, Java, and Go.
Testing: the full package suite completed with 360 passed and 271 skipped. The final worktree-bound batch-delete suite passes 44 tests.
api.mdandapi.metadata.ymlwere regenerated from the local package.All SDK Contribution checklist