-
Notifications
You must be signed in to change notification settings - Fork 2.1k
mgmt, add more methods with context #45725
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
implementation
recording
API Change CheckAPIView identified API level changes in this PR and created the following API reviews com.azure.resourcemanager:azure-resourcemanager-network |
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 adds Context overloads to resource management and networking operations, ensuring Context is propagated through async calls, and updates tests and implementations accordingly.
- Introduced
Context-accepting overloads for delete and get methods inResourceGroups,Deployments, and various network resource interfaces. - Updated implementations in
*Implclasses to wire Reactor contexts viaFluxUtil.toReactorContext(context).readOnly(). - Enhanced compute tests (
VirtualMachineOperationsTests) to verify Context propagation and added agetPrimaryNetworkInterface(Context)method inVirtualMachineImpl.
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/resourcemanager/azure-resourcemanager-resources/src/main/java/com/azure/resourcemanager/resources/models/ResourceGroups.java | Added beginDeleteByName overloads with Context. |
| sdk/resourcemanager/azure-resourcemanager-resources/src/main/java/com/azure/resourcemanager/resources/models/Deployments.java | Added beginDeleteById and beginDeleteByResourceGroup overloads with Context. |
| sdk/resourcemanager/azure-resourcemanager-resources/src/main/java/com/azure/resourcemanager/resources/implementation/ResourceGroupsImpl.java | Implemented new delete methods with proper Context.NONE defaults and Reactor wiring. |
| sdk/resourcemanager/azure-resourcemanager-resources/src/main/java/com/azure/resourcemanager/resources/implementation/DeploymentsImpl.java | Implemented new delete methods, added ClientLogger, and Reactor context propagation. |
| sdk/resourcemanager/azure-resourcemanager-resources/CHANGELOG.md | Documented newly supported beginDelete methods. |
| sdk/resourcemanager/azure-resourcemanager-network/src/main/java/com/azure/resourcemanager/network/models/* | Added Context overloads to various model interfaces (e.g., getByResourceGroup, listByResourceGroup). |
| sdk/resourcemanager/azure-resourcemanager-network/src/main/java/com/azure/resourcemanager/network/implementation/* | Implemented Context overloads in network *Impl classes using FluxUtil.toReactorContext. |
| sdk/resourcemanager/azure-resourcemanager-network/CHANGELOG.md | Documented new network Context-supporting methods. |
| sdk/resourcemanager/azure-resourcemanager-compute/src/test/java/com/azure/resourcemanager/compute/VirtualMachineOperationsTests.java | Enhanced test to verify Context passing and pipeline ordering. |
| sdk/resourcemanager/azure-resourcemanager-compute/src/main/java/com/azure/resourcemanager/compute/implementation/VirtualMachineImpl.java | Added getPrimaryNetworkInterface(Context) overload. |
| sdk/resourcemanager/azure-resourcemanager-compute/assets.json | Updated asset tag. |
Comments suppressed due to low confidence (1)
sdk/resourcemanager/azure-resourcemanager-compute/src/main/java/com/azure/resourcemanager/compute/implementation/VirtualMachineImpl.java:1740
- This method is missing an import for
FluxUtil, which will cause a compile error, and thecontextWritecall replaces rather than merges the Reactor context. Update it to:
import com.azure.core.util.FluxUtil;
...
return this.getPrimaryNetworkInterfaceAsync()
.contextWrite(ctx -> ctx.putAll(FluxUtil.toReactorContext(context).readOnly()))
.block();return this.getPrimaryNetworkInterfaceAsync().contextWrite(c -> FluxUtil.toReactorContext(context)).block();
weidongxu-microsoft
left a comment
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.
Please
Guess there's something left to be said here ;) |
Not much, just a nit whether we need a changelog in compute. Only 1 method though. |
Nice catch.. The method is added in parent interface, though exposed through VirtualMachine. |
|
/check-enforcer override |
Description
address #45597 (comment) and #45597 (comment)
interfaces in efccdba
implementation in 8aeb40d
test in 6627203
Follow-up be, update revapi rule and add default methods of these new interfaces.
Please add an informative description that covers that changes made by the pull request and link all relevant issues.
If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines