Skip to content
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

Update Generated Types #838

Closed
wants to merge 13 commits into from
Closed

Update Generated Types #838

wants to merge 13 commits into from

Conversation

github-actions[bot]
Copy link
Contributor

Update Generated Types

Generate types for https://github.com/Azure/azure-rest-api-specs/tree/9722d269ce8ad6bf8de7e8083f0409e8bcb0569f

Summary

Failed to generate types for path 'adhybridhealthservice'
Exited with code 1
Failed to generate types for path 'securityinsights'
Exited with code 1
Failed to generate types for path 'service-map'
Exited with code 1
Failed to generate types for path 'timeseriesinsights'
Exited with code 1
Failed to generate types for path 'web'
Exited with code 1

@jeskew jeskew marked this pull request as draft May 26, 2022 19:53
@jeskew jeskew force-pushed the autogenerate-f80bcf2d branch from 676295a to b0ce568 Compare May 27, 2022 22:48
@jeskew jeskew force-pushed the jeskew/add-resource-tags branch from 0e3d296 to c74a5d5 Compare June 8, 2022 21:24
@jeskew jeskew force-pushed the autogenerate-f80bcf2d branch from b0ce568 to f842d2b Compare June 8, 2022 23:42
@jeskew jeskew force-pushed the jeskew/add-resource-tags branch from c74a5d5 to ace36df Compare June 16, 2022 19:51
@jeskew jeskew force-pushed the autogenerate-f80bcf2d branch from f842d2b to c9f80d1 Compare June 17, 2022 18:14
generated/index.md Outdated Show resolved Hide resolved
generated/index.md Outdated Show resolved Hide resolved
generated/index.md Outdated Show resolved Hide resolved
@jeskew jeskew force-pushed the autogenerate-f80bcf2d branch 3 times, most recently from f13caf7 to 32973d5 Compare June 21, 2022 17:31
Comment on lines -1372 to +1501
* **hostingEnvironment**: string: App Service Environment name, if needed (only when restoring a site to an App Service Environment)
* **ignoreConflictingHostNames**: bool: Changes a logic when restoring a site with custom domains. If "true", custom domains are removed automatically. If "false", custom domains are added to
* **finishedTimeStamp**: string (ReadOnly): Timestamp when this backup finished.
* **hostingEnvironment**: string (WriteOnly): App Service Environment name, if needed (only when restoring a site to an App Service Environment)
* **id**: int (ReadOnly): Id of the backup.
* **ignoreConflictingHostNames**: bool (WriteOnly): Changes a logic when restoring a site with custom domains. If "true", custom domains are removed automatically. If "false", custom domains are added to
the site object when it is being restored, but that might fail due to conflicts during the operation.
* **operationType**: 'Clone' | 'Default' | 'Relocation' (Required): Operation type
* **overwrite**: bool: True if the restore operation can overwrite target site. "True" needed if trying to restore over an existing site.
* **siteName**: string: Name of a site (Web App)
* **lastRestoreTimeStamp**: string (ReadOnly): Timestamp of a last restore operation which used this backup.
* **log**: string (ReadOnly): Details regarding this backup. Might contain an error message.
* **name**: string (ReadOnly): Name of this backup
* **operationType**: 'Clone' | 'Default' | 'Relocation' (Required, WriteOnly): Operation type
* **overwrite**: bool (WriteOnly): True if the restore operation can overwrite target site. "True" needed if trying to restore over an existing site.
* **scheduled**: bool (ReadOnly): True if this backup has been created due to a schedule being triggered.
* **siteName**: string (WriteOnly): Name of a site (Web App)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No properties were removed, but the ones marked as line deletions only exist on the PUT request and are now flagged as WriteOnly.

The particular resource that uses this synthetic object definition had the PUT and GET defined as distinct paths, one with a constant name and the other with a parameterized name.

Comment on lines -1024 to +1446
## RestoreRequestProperties
## RestoreRequestPropertiesOrBackupItemProperties
### Properties
* **adjustConnectionStrings**: bool: <code>true</code> if SiteConfig.ConnectionStrings should be set in new app; otherwise, <code>false</code>.
* **appServicePlan**: string: Specify app service plan that will own restored site.
* **adjustConnectionStrings**: bool (WriteOnly): <code>true</code> if SiteConfig.ConnectionStrings should be set in new app; otherwise, <code>false</code>.
* **appServicePlan**: string (WriteOnly): Specify app service plan that will own restored site.
* **blobName**: string: Name of a blob which contains the backup.
* **correlationId**: string (ReadOnly): Unique correlation identifier. Please use this along with the timestamp while communicating with Azure support.
* **created**: string (ReadOnly): Timestamp of the backup creation.
* **databases**: [DatabaseBackupSetting](#databasebackupsetting)[]: Collection of databases which should be restored. This list has to match the list of databases included in the backup.
* **hostingEnvironment**: string: App Service Environment name, if needed (only when restoring an app to an App Service Environment).
* **ignoreConflictingHostNames**: bool: Changes a logic when restoring an app with custom domains. <code>true</code> to remove custom domains automatically. If <code>false</code>, custom domains are added to
* **finishedTimeStamp**: string (ReadOnly): Timestamp when this backup finished.
* **hostingEnvironment**: string (WriteOnly): App Service Environment name, if needed (only when restoring an app to an App Service Environment).
* **id**: int (ReadOnly): Id of the backup.
* **ignoreConflictingHostNames**: bool (WriteOnly): Changes a logic when restoring an app with custom domains. <code>true</code> to remove custom domains automatically. If <code>false</code>, custom domains are added to
the app's object when it is being restored, but that might fail due to conflicts during the operation.
* **ignoreDatabases**: bool: Ignore the databases and only restore the site content
* **operationType**: 'Clone' | 'Default' | 'Relocation' | 'Snapshot': Operation type.
* **overwrite**: bool (Required): <code>true</code> if the restore operation can overwrite target app; otherwise, <code>false</code>. <code>true</code> is needed if trying to restore over an existing app.
* **siteName**: string: Name of an app.
* **ignoreDatabases**: bool (WriteOnly): Ignore the databases and only restore the site content
* **lastRestoreTimeStamp**: string (ReadOnly): Timestamp of a last restore operation which used this backup.
* **log**: string (ReadOnly): Details regarding this backup. Might contain an error message.
* **name**: string (ReadOnly): Name of this backup.
* **operationType**: 'Clone' | 'Default' | 'Relocation' | 'Snapshot' (WriteOnly): Operation type.
* **overwrite**: bool (Required, WriteOnly): <code>true</code> if the restore operation can overwrite target app; otherwise, <code>false</code>. <code>true</code> is needed if trying to restore over an existing app.
* **scheduled**: bool (ReadOnly): True if this backup has been created due to a schedule being triggered.
* **siteName**: string (WriteOnly): Name of an app.
* **sizeInBytes**: int (ReadOnly): Size of the backup in bytes.
* **status**: 'Created' | 'DeleteFailed' | 'DeleteInProgress' | 'Deleted' | 'Failed' | 'InProgress' | 'PartiallySucceeded' | 'Skipped' | 'Succeeded' | 'TimedOut' (ReadOnly): Backup status.
* **storageAccountUrl**: string (Required): SAS URL to the container.
* **websiteSizeInBytes**: int (ReadOnly): Size of the original web app which has been backed up.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same behavior as is demonstrated in 2015-08-01 version.

@jeskew jeskew force-pushed the autogenerate-f80bcf2d branch from 32973d5 to 9ae3ddf Compare June 22, 2022 00:00
@jeskew jeskew force-pushed the autogenerate-f80bcf2d branch from 9ae3ddf to 81795ce Compare June 22, 2022 02:43
@jeskew jeskew force-pushed the autogenerate-f80bcf2d branch from 81795ce to df46713 Compare June 22, 2022 19:34
* **name**: 'DirectLineChannel' | 'EmailChannel' | 'FacebookChannel' | 'KikChannel' | 'MsTeamsChannel' | 'SkypeChannel' | 'SlackChannel' | 'SmsChannel' | 'TelegramChannel' | 'WebChatChannel' (Required, DeployTimeConstant): The resource name
* **name**: 'DirectLineChannel' | 'EmailChannel' | 'FacebookChannel' | 'KikChannel' | 'MsTeamsChannel' | 'SkypeChannel' | 'SlackChannel' | 'SmsChannel' | 'TelegramChannel' | 'WebChatChannel' | string (Required, DeployTimeConstant): The resource name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The GET side of this resource allows any string to be used as the name parameter. We were previously only looking at the PUT side, which uses a closed enum.

Comment on lines -114 to +162
* **name**: 'current' (Required, DeployTimeConstant): The resource name
* **name**: 'current' | 'recommended' (Required, DeployTimeConstant): The resource name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, this resource is a bit weird. The GET side of the resource allows 'current' or 'recommended', which I assume correspond to "tell me what sensitivity labels are in use" and "tell me what sensitivity labels you think I should be using," respectively. The PUT side only allows 'current', because this is essentially a singleton child resource of Microsoft.Sql/servers/databases/schemas/tables/columns. The updates needed to support GET and PUT sides of the same resource defined at slightly differing paths requires the type generator to look at how a resource's name is defined on both GET and PUT and reconcile them, which has the interesting side effect we see here.

In an ideal world, I would probably want there to be a POST action on Microsoft.Sql/servers/databases/schemas/tables/columns called something like listRecommendedSensitivityLabels instead of having the provider overload the GET side of the resource like this. But I'm OK with the end result, even if it is a bit unexpected.

Comment on lines -217 to +328
* **name**: 'current' (Required, DeployTimeConstant): The resource name
* **name**: 'current' | 'recommended' (Required, DeployTimeConstant): The resource name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as observed in sql path with Microsoft.Sql/servers/databases/schemas/tables/columns/sensitivityLabels resource.

@jeskew jeskew force-pushed the autogenerate-f80bcf2d branch from df46713 to 74ef915 Compare June 23, 2022 02:32
@jeskew jeskew force-pushed the autogenerate-f80bcf2d branch from 74ef915 to d43e6b5 Compare June 23, 2022 16:31
Comment on lines +3 to +11
## Resource Microsoft.Billing/billingPeriods/Microsoft.Consumption@2018-06-30 (ReadOnly)
* **Valid Scope(s)**: ManagementGroup
### Properties
* **apiVersion**: '2018-06-30' (ReadOnly, DeployTimeConstant): The resource api version
* **id**: string (ReadOnly, DeployTimeConstant): The resource id
* **name**: 'aggregatedcost' (Required, DeployTimeConstant): The resource name
* **properties**: [ManagementGroupAggregatedCostProperties](#managementgroupaggregatedcostproperties) (ReadOnly): The properties of the Management Group Aggregated Cost.
* **tags**: [ResourceTags](#resourcetags) (ReadOnly): Resource tags.
* **type**: 'Microsoft.Billing/billingPeriods/Microsoft.Consumption' (ReadOnly, DeployTimeConstant): The resource type
Copy link
Contributor

@jeskew jeskew Jun 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that this is wrong, but the name certainly looks weird. The RP method does follow the hierarchical resource naming pattern, and the response body is tagged by the service as a resource. My guess is that this is an error in the Swagger (i.e., there is a missing /providers/ path segment that should precede Microsoft.Consumption) that accidentally happened to qualify as a resource.

Comment on lines -8 to 10
* **name**: 'default' (Required, DeployTimeConstant): The resource name
* **name**: 'default' | string (Required, DeployTimeConstant): The resource name
* **properties**: [ApplyUpdateProperties](#applyupdateproperties) (ReadOnly): Properties of the apply update
* **type**: 'Microsoft.Maintenance/applyUpdates' (ReadOnly, DeployTimeConstant): The resource type
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This resource defines its PUT with a constant name (the path ends in /default) and uses a parameterized path for the GET method targeting the same resource type at the same scope. What's being generated here is not incorrect IME, but this is an interesting collateral effect.

Comment on lines -145 to +183
* **name**: 'discover' (Required, DeployTimeConstant): The resource name
* **properties**: [RestoreRequestProperties](#restorerequestproperties)
* **name**: 'discover' | string (Required, DeployTimeConstant): The resource name
* **properties**: [RestoreRequestPropertiesOrBackupItemProperties](#restorerequestpropertiesorbackupitemproperties)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is another case where the PUT side uses a constant string name and the GET side uses a parameterized name.

Looking at the semantics of the PUT method that's currently being captured as a resource, I believe this is an RP modeling error. The PUT method should be a POST action, and the /backups resource should be read only. That'd be a breaking change and would need to be fixed in the swagger.

@jeskew jeskew force-pushed the autogenerate-f80bcf2d branch from ac6996c to 008f8ed Compare June 24, 2022 17:36
Update Generated Types 

Generate types for https://github.com/Azure/azure-rest-api-specs/tree/d006e1d15d8fe19a6c558861dd5bba6b7baa8dd8

Summary
<details>
  <summary>Failed to generate types for path 'adhybridhealthservice'</summary>

```
Exited with code 1
```
</details>

<details>
  <summary>Failed to generate types for path 'service-map'</summary>

```
Exited with code 1
```
</details>
@jeskew jeskew force-pushed the autogenerate-f80bcf2d branch from 008f8ed to 9656b9c Compare June 24, 2022 20:38
Base automatically changed from jeskew/add-resource-tags to main June 24, 2022 23:30
@jeskew jeskew closed this Jun 24, 2022
@jeskew jeskew deleted the autogenerate-f80bcf2d branch March 30, 2023 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants