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.

Sign up for GitHub

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

Jump to bottom

Adding a legacy Required properties resource mix models #833

Closed
wants to merge 2 commits into from
Closed

Adding a legacy Required properties resource mix models #833

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d09d81a
Adding a legacy Required properties resource mix models
allenjzhang May 10, 2024
99580c9
Merge branch 'main' of https://github.com/Azure/typespec-azure into a…
allenjzhang May 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/typespec-azure-resource-manager/lib/arm.tsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import "./common-types/managed-identity.tsp";
import "./common-types/private-links.tsp";
import "./common-types/customer-managed-keys.tsp";
import "./common-types/extended-location.tsp";
import "./legacy/models.tsp";
import "./private.decorators.tsp";
import "./models.tsp";
import "./operations.tsp";
Expand Down
59 changes: 59 additions & 0 deletions packages/typespec-azure-resource-manager/lib/legacy/models.tsp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
using TypeSpec.Http;
Copy link
Member

Choose a reason for hiding this comment

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

For legacy, I think we are still leaving out scenarios where folks extend Resource or EntityResource. I would prefer it if we just let them extend a common-types resource type, and added a new decorator they would need to decorate the type with that is the equivalent of @armResourceInternal and @includeInapplicableMetadataInPayload, and relax the rules around extension to include this case.

using TypeSpec.Rest;
using TypeSpec.OpenAPI;
using Azure.ResourceManager.Foundations;
using Azure.ResourceManager.Private;

namespace Azure.ResourceManager.Legacy;

//#region Standard Resource Operation Interfaces
Copy link
Member

Choose a reason for hiding this comment

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

do we supprot region in tsp?

Copy link
Member

Choose a reason for hiding this comment

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

remove

/**
* Concrete tracked resource types can be created by aliasing this type using a specific property type.
*
* See more details on [different Azure Resource Manager resource type here.](https://azure.github.io/typespec-azure/docs/howtos/ARM/resource-type)
* @template Properties A model containing the provider-specific properties for this resource
*/
@doc("Concrete tracked resource types can be created by aliasing this type using a specific property type.")
markcowl marked this conversation as resolved.
Show resolved Hide resolved
@armResourceInternal(Properties)
@includeInapplicableMetadataInPayload(false)
model TrackedResourceWithRequiredProperties<Properties extends {}>
Copy link
Member

Choose a reason for hiding this comment

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

Should we put #deprecated on all of those or have a linting rule against using them?

Copy link
Member Author

Choose a reason for hiding this comment

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

I feel the namespace being legacy which has good separation. It is targeted for brownfield. Unless ARM has disallowed required properties (currently they don't) we probably don't want to put deprecate.

extends Foundations.TrackedResource {
@doc("The resource-specific required properties for this resource.")
@extension("x-ms-client-flatten", true)
properties: Properties;
}

/**
* Concrete proxy resource types can be created by aliasing this type using a specific property type.
*
* See more details on [different Azure Resource Manager resource type here.](https://azure.github.io/typespec-azure/docs/howtos/ARM/resource-type)
* @template Properties A model containing the provider-specific properties for this resource
*/
@doc("Concrete proxy resource types can be created by aliasing this type using a specific property type.")
@armResourceInternal(Properties)
@includeInapplicableMetadataInPayload(false)
model ProxyResourceWithRequiredProperties<Properties extends {}> extends Foundations.ProxyResource {
@doc("The resource-specific required properties for this resource.")
@extension("x-ms-client-flatten", true)
properties: Properties;
}

/**
* Concrete extension resource types can be created by aliasing this type using a specific property type.
*
* See more details on [different Azure Resource Manager resource type here.](https://azure.github.io/typespec-azure/docs/howtos/ARM/resource-type)
* @template Properties A model containing the provider-specific properties for this resource
*/
@extensionResource
@doc("Concrete extension resource types can be created by aliasing this type using a specific property type.")
@armResourceInternal(Properties)
@includeInapplicableMetadataInPayload(false)
model ExtensionResourceWithRequiredProperties<Properties extends {}>
extends Foundations.ExtensionResource {
@doc("The resource-specific required properties for this resource.")
@extension("x-ms-client-flatten", true)
properties: Properties;
}
//#region

//#region Standard extraction models
6 changes: 3 additions & 3 deletions packages/typespec-azure-resource-manager/lib/models.tsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ model ResourceNameParameter<
@armResourceInternal(Properties)
@includeInapplicableMetadataInPayload(false)
model TrackedResource<Properties extends {}> extends Foundations.TrackedResource {
@doc("The resource-specific properties for this resource.")
@doc("The resource-specific optional properties for this resource.")
Copy link
Member

Choose a reason for hiding this comment

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

I don't think these doc updates are necessary. The typespec and the swagger already say it is optional, adding the documentation makes swagger changes without adding any information.

@visibility("read", "create")
@extension("x-ms-client-flatten", true)
properties?: Properties;
Expand All @@ -56,7 +56,7 @@ model TrackedResource<Properties extends {}> extends Foundations.TrackedResource
@armResourceInternal(Properties)
@includeInapplicableMetadataInPayload(false)
model ProxyResource<Properties extends {}> extends Foundations.ProxyResource {
@doc("The resource-specific properties for this resource.")
@doc("The resource-specific optional properties for this resource.")
@visibility("read", "create")
@extension("x-ms-client-flatten", true)
properties?: Properties;
Expand All @@ -73,7 +73,7 @@ model ProxyResource<Properties extends {}> extends Foundations.ProxyResource {
@armResourceInternal(Properties)
@includeInapplicableMetadataInPayload(false)
model ExtensionResource<Properties extends {}> extends Foundations.ExtensionResource {
@doc("The resource-specific properties for this resource.")
@doc("The resource-specific optional properties for this resource.")
@visibility("read", "create")
@extension("x-ms-client-flatten", true)
properties?: Properties;
Expand Down
Loading