-
Notifications
You must be signed in to change notification settings - Fork 5.7k
adding mutability flag to the identity property ACS #24477
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -686,7 +686,12 @@ | |
| "x-ms-client-flatten": true | ||
| }, | ||
| "identity": { | ||
| "$ref": "../../../../../common-types/resource-management/v5/managedidentity.json#/definitions/ManagedServiceIdentity" | ||
| "$ref": "../../../../../common-types/resource-management/v5/managedidentity.json#/definitions/ManagedServiceIdentity", | ||
| "x-ms-mutability": [ | ||
| "create", | ||
| "read", | ||
| "update" | ||
| ] | ||
|
Comment on lines
+690
to
+694
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see why you need this, as by default that's the 3 values... But anyway I guess it won't hurt.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's when readonly property is indicated, also autorest doesn't create builder methods for java to allow create or update when that isn't specified.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But I didn't see anywhere in swagger stating that "identity" property is "readonly". The property is not in parrent "TrackedResource". And here it does not have
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a third option when it's just null considering it's a JSON object, the fact that readonly isn't specified doesn't mean false is applied by default. Either case , this gave us our expected behavior using autorest for generating java package.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Emm, I am not sure about it. I actually works on Java codegen, and a quick run on the spec (without this change) works fine. // Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
// Code generated by Microsoft (R) AutoRest Code Generator.
package com.azure.resourcemanager.communication.generated;
import com.azure.resourcemanager.communication.models.ManagedServiceIdentity;
import com.azure.resourcemanager.communication.models.ManagedServiceIdentityType;
/** Samples for CommunicationServices CreateOrUpdate. */
public final class CommunicationServicesCreateOrUpdateSamples {
/*
* x-ms-original-file: specification/communication/resource-manager/Microsoft.Communication/preview/2023-04-01-preview/examples/communicationServices/createOrUpdate.json
*/
/**
* Sample code: Create or update resource.
*
* @param manager Entry point to CommunicationManager.
*/
public static void createOrUpdateResource(com.azure.resourcemanager.communication.CommunicationManager manager) {
manager
.communicationServices()
.define("MyCommunicationResource")
.withRegion("Global")
.withExistingResourceGroup("MyResourceGroup")
.withDataLocation("United States")
.create();
}
/*
* x-ms-original-file: specification/communication/resource-manager/Microsoft.Communication/preview/2023-04-01-preview/examples/communicationServices/createOrUpdateWithSystemAssignedIdentity.json
*/
/**
* Sample code: Create or update resource with managed identity.
*
* @param manager Entry point to CommunicationManager.
*/
public static void createOrUpdateResourceWithManagedIdentity(
com.azure.resourcemanager.communication.CommunicationManager manager) {
manager
.communicationServices()
.define("MyCommunicationResource")
.withRegion("Global")
.withExistingResourceGroup("MyResourceGroup")
.withIdentity(new ManagedServiceIdentity().withType(ManagedServiceIdentityType.SYSTEM_ASSIGNED))
.withDataLocation("United States")
.create();
}
}
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The update sample seems not correct, but code should still work if one do
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting, this is worth investigating as 2 other people also don't have the withIdentity method generated.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Use The full command is pretty complicated. See https://github.com/Azure/azure-sdk-for-java/wiki/%5BManagement%5D--OpenAPI-specs for automated codegen.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem in SDK sample is caused by the error in swagger example JSON. The Please fix in swagger, if you have the chance. |
||
| } | ||
| } | ||
| }, | ||
|
|
||
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.
2020-08-20-preview has gone through the full deprecation announcement process and RP and ARM no longer accepts requests for this API Version