Run local typegen for comparison #885
Conversation
jeskew
force-pushed
the
jeskew/update-named-type-property-flagging
branch
from
ee506d2 to
04868f8
Compare
| * **principalId**: string (ReadOnly, WriteOnly): The identity id | ||
| * **principalName**: string (ReadOnly, WriteOnly): The identity display name | ||
| * **principalType**: 'servicePrincipal' | 'user' | string (ReadOnly, WriteOnly): The identity type : user/servicePrincipal | ||
| * **userPrincipalName**: string (ReadOnly, WriteOnly): The user principal name(if valid) | ||
| * **principalId**: string (ReadOnly): The identity id | ||
| * **principalName**: string (ReadOnly): The identity display name | ||
| * **principalType**: 'servicePrincipal' | 'user' | string (ReadOnly): The identity type : user/servicePrincipal | ||
| * **userPrincipalName**: string (ReadOnly): The user principal name(if valid) |
There was a problem hiding this comment.
Properties can currently end up flagged as both ReadOnly and WriteOnly when the readOnly swagger attribute on the PUT property is set to true and no GET property exists. This outcome still occurs a handful of times after #884, but only on misconfigured resource properties (e.g., when a property is marked as readOnly on the PUT request and absent from the GET response).
| * **primaryConnectionString**: string (ReadOnly): Connection string constructed via the primaryKey | ||
| * **primaryKey**: string (ReadOnly): The primary access key. | ||
| * **secondaryConnectionString**: string (ReadOnly): Connection string constructed via the secondaryKey | ||
| * **secondaryKey**: string (ReadOnly): The secondary access key. | ||
| * **primaryConnectionString**: string (WriteOnly): Connection string constructed via the primaryKey | ||
| * **primaryKey**: string (WriteOnly): The primary access key. | ||
| * **secondaryConnectionString**: string (WriteOnly): Connection string constructed via the secondaryKey | ||
| * **secondaryKey**: string (WriteOnly): The secondary access key. |
There was a problem hiding this comment.
This update may raise some backwards compatibility concerns. The WebPubSubKeys object is only returned by the Microsoft.SignalRService/webPubSub::listKeys function, so its properties were previously all flagged as ReadOnly. Each property has an x-ms-mutability extension of ['create', 'update'], so the WriteOnly flag is being correctly applied.
This change only affects two preview API versions; the stable API version that defines this function (Microsoft.SignalRService@2021-10-01) does not flag the properties of this return object as WriteOnly as of Azure/azure-rest-api-specs#17228
| @@ -1010,6 +1010,43 @@ | |||
| * **vnetSubnetName**: string: Subnet of the hostingEnvironment's (App Service Environment) virtual network | |||
| * **workerPools**: [WorkerPool](#workerpool)[]: Description of worker pools with worker size ids, VM sizes, and number of workers in each pool | |||
|
|
|||
| ## HostingEnvironmentPropertiesOrManagedHostingEnvironmentProperties | |||
There was a problem hiding this comment.
This new object definition comes from a resource (Microsoft.Web/managedHostingEnvironments) whose PUT request body shape is also used by another resource (Microsoft.Web/hostingEnvironments). The latter resource uses the same shape for the PUT request and GET response bodies and is defined earlier in the Swagger model than the former resource, so ReadOnly/WriteOnly hints for the latter resource were not previously available despite the PUT request and GET response bodies having different definitions.
| ## CertificateCreateOrUpdateProperties | ||
| ## CertificateCreateOrUpdatePropertiesOrCertificateContractProperties |
There was a problem hiding this comment.
My assumption here is that given Bicep's (and AZ terraform's) structural typing system, renaming an object is not something that would affect users. This may negatively impact any links to anchors within the ARM template reference docs since the anchor for an object is just the object name in lowercase. Links to missing anchors will still land on the correct page, though, so I'm not sure it's that impactful.
| * **externalId**: string (ReadOnly): For external groups, this property contains the id of the group from the external identity provider, e.g. for Azure Active Directory aad://<tenant>.onmicrosoft.com/groups/<group object id>; otherwise the value is null. | ||
| * **type**: 'custom' | 'external' | 'system' (ReadOnly): Group type. | ||
| * **description**: string: Group description. Can contain HTML formatting tags. | ||
| * **displayName**: string (Required): Group name. |
There was a problem hiding this comment.
I don't believe this flag transition (ReadOnly -> Required) is a backwards incompatible change, as it only occurs in this PR on properties of objects that are themselves only accessible in the context of a read. The type generator suppresses the Required flag on properties that only exist within a GET, and this PR updates that behavior to only occur on top-level resource properties and on paths where the PUT request and GET response resolve to different object definitions.
Semantically, I think this may mean that the displayName field is always present while other properties (e.g., externalId on this object) may be absent.
generated/recoveryservicessiterecovery/microsoft.recoveryservices/2021-03-01/types.md
Outdated
Show resolved
Hide resolved
| * **primaryConnectionString**: string (ReadOnly): Connection string constructed via the primaryKey | ||
| * **primaryKey**: string (ReadOnly): The primary access key. | ||
| * **secondaryConnectionString**: string (ReadOnly): Connection string constructed via the secondaryKey | ||
| * **secondaryKey**: string (ReadOnly): The secondary access key. | ||
| * **primaryConnectionString**: string (WriteOnly): Connection string constructed via the primaryKey | ||
| * **primaryKey**: string (WriteOnly): The primary access key. | ||
| * **secondaryConnectionString**: string (WriteOnly): Connection string constructed via the secondaryKey | ||
| * **secondaryKey**: string (WriteOnly): The secondary access key. |
There was a problem hiding this comment.
Same issue as came up on webpubsub's WebPubSubKeys shape.
| ## PurchaseRequestProperties | ||
| ### Properties | ||
| * **appliedScopes**: string[]: List of the subscriptions that the benefit will be applied. Do not specify if AppliedScopeType is Shared. | ||
| * **appliedScopeType**: 'Shared' | 'Single' | string: Type of the Applied Scope. | ||
| * **billingPlan**: 'Monthly' | 'Upfront' | string: Represent the billing plans. | ||
| * **billingScopeId**: string: Subscription that will be charged for purchasing Reservation | ||
| * **displayName**: string: Friendly name of the Reservation | ||
| * **quantity**: int: Quantity of the SKUs that are part of the Reservation. Must be greater than zero. | ||
| * **renew**: bool: Setting this to true will automatically purchase a new reservation on the expiration date time. | ||
| * **reservedResourceProperties**: [PurchaseRequestPropertiesReservedResourceProperties](#purchaserequestpropertiesreservedresourceproperties): Properties specific to each reserved resource type. Not required if not applicable. | ||
| * **reservedResourceType**: 'AppService' | 'AzureDataExplorer' | 'BlockBlob' | 'CosmosDb' | 'Databricks' | 'DedicatedHost' | 'ManagedDisk' | 'MariaDb' | 'MySql' | 'PostgreSql' | 'RedHat' | 'RedHatOsa' | 'RedisCache' | 'SapHana' | 'SqlAzureHybridBenefit' | 'SqlDataWarehouse' | 'SqlDatabases' | 'SuseLinux' | 'VMwareCloudSimple' | 'VirtualMachines' | string: The type of the resource that is being reserved. | ||
| * **term**: 'P1Y' | 'P3Y' | string: Represent the term of Reservation. | ||
|
|
||
| ## PurchaseRequestPropertiesOrReservationOrderProperties | ||
| ### Properties | ||
| * **appliedScopes**: string[] (WriteOnly): List of the subscriptions that the benefit will be applied. Do not specify if AppliedScopeType is Shared. | ||
| * **appliedScopeType**: 'Shared' | 'Single' | string (WriteOnly): Type of the Applied Scope. | ||
| * **billingPlan**: 'Monthly' | 'Upfront' | string: Represent the billing plans. |
There was a problem hiding this comment.
This is the best example I've encountered so far of why #884 is necessary. PurchaseRequestProperties is used in multiple resources, and the generated type for that object on main is a combination of PurchaseRequestProperties and ReservationOrderProperties, with ReadOnly and WriteOnly flags sprinkled on properties based on usage in one of the aforementioned multiple resources.
I saw this same issue come up repeatedly in #805, but it is less common when only types for writable resources are generated.
jeskew
force-pushed
the
jeskew/update-named-type-property-flagging
branch
from
04868f8 to
c9fb2fb
Compare
jeskew
force-pushed
the
jeskew/update-named-type-property-flagging-typegen
branch
from
76fddc8 to
7e1fe35
Compare
Companion to #884