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

add resource type scalar in typespec-azure-core #1272

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
+41 −7
Open

add resource type scalar in typespec-azure-core #1272

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b3c0834
add resource type scalar and put it in some corresponding places
ArcturusZhang Jul 31, 2024
a95653d
add changelog
ArcturusZhang Jul 31, 2024
ac93669
update the gen sample to ensure it does not change
ArcturusZhang Jul 31, 2024
de619ce
rebuild
ArcturusZhang Jul 31, 2024
a3c466c
Merge branch 'main' into add-resource-type
ArcturusZhang Jul 31, 2024
2b68cf3
Merge branch 'main' into add-resource-type
ArcturusZhang Aug 7, 2024
3494df5
Merge branch 'main' into add-resource-type
ArcturusZhang Aug 7, 2024
47aa996
Merge remote-tracking branch 'origin/main' into add-resource-type
ArcturusZhang Sep 5, 2024
914fb65
Update .chronus/changes/add-resource-type-2024-6-31-14-6-51.md
ArcturusZhang Sep 6, 2024
840aa34
Merge branch 'main' into add-resource-type
ArcturusZhang Sep 6, 2024
a1a3730
Merge branch 'main' into add-resource-type
ArcturusZhang Sep 19, 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
8 changes: 8 additions & 0 deletions .chronus/changes/add-resource-type-2024-6-31-14-6-51.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
changeKind: feature
packages:
- "@azure-tools/typespec-azure-core"
- "@azure-tools/typespec-azure-resource-manager"
---

Add resourceType scalar, and changed some properties to be resourceType instead of string
ArcturusZhang marked this conversation as resolved.
Show resolved Hide resolved
22 changes: 18 additions & 4 deletions docs/libraries/azure-core/reference/data-types.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,10 @@ model Azure.Core.ArmResourceIdentifierAllowedResource

#### Properties

| Name | Type | Description |
| ------- | ----------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| type | `string` | The type of resource that is being referred to. For example Microsoft.Network/virtualNetworks or Microsoft.Network/virtualNetworks/subnets. See Example Types for more examples. |
| scopes? | `Core.ArmResourceDeploymentScope[]` | An array of scopes. If not specified, the default scope is ["ResourceGroup"].<br />See [Allowed Scopes](https://github.com/Azure/autorest/tree/main/docs/extensions#allowed-scopes). |
| Name | Type | Description |
| ------- | ------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| type | [`resourceType`](#Azure.Core.resourceType) | The type of resource that is being referred to. For example Microsoft.Network/virtualNetworks or Microsoft.Network/virtualNetworks/subnets. See Example Types for more examples. |
| scopes? | `Core.ArmResourceDeploymentScope[]` | An array of scopes. If not specified, the default scope is ["ResourceGroup"].<br />See [Allowed Scopes](https://github.com/Azure/autorest/tree/main/docs/extensions#allowed-scopes). |

### `AzureApiKeyAuthentication` {#Azure.Core.AzureApiKeyAuthentication}

Expand Down Expand Up @@ -604,6 +604,20 @@ scalar Azure.Core.ipV6Address
2001:db8:3333:4444:CCCC:DDDD:EEEE:FFFF
```

### `resourceType` {#Azure.Core.resourceType}

Represents an Azure Resource Type.

```typespec
scalar Azure.Core.resourceType
```

#### Examples

```
Microsoft.Network/virtualNetworks/subnets
```

### `uuid` {#Azure.Core.uuid}

Universally Unique Identifier
Expand Down
2 changes: 1 addition & 1 deletion docs/libraries/azure-resource-manager/reference/data-types.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1546,7 +1546,7 @@ model Azure.ResourceManager.CommonTypes.Resource
| ----------- | ---------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| id? | `Core.armResourceIdentifier` | Fully qualified resource ID for the resource. Ex - /subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/{resourceProviderNamespace}/{resourceType}/{resourceName} |
| name? | `string` | The name of the resource |
| type? | `string` | The type of the resource. E.g. "Microsoft.Compute/virtualMachines" or "Microsoft.Storage/storageAccounts" |
| type? | `Core.resourceType` | The type of the resource. E.g. "Microsoft.Compute/virtualMachines" or "Microsoft.Storage/storageAccounts" |
| systemData? | [`SystemData`](./data-types.md#Azure.ResourceManager.CommonTypes.SystemData) | Azure Resource Manager metadata containing createdBy and modifiedBy information. |

### `ResourceGroupNameParameter` {#Azure.ResourceManager.CommonTypes.ResourceGroupNameParameter}
Expand Down
1 change: 1 addition & 0 deletions packages/samples/common-types/gen.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ function cleanupDocument(original: OpenAPI2Document): OpenAPI2Document {

replaceUuidRefs(document, "Azure.Core.uuid");
replaceUuidRefs(document, "Azure.Core.azureLocation");
replaceUuidRefs(document, "Azure.Core.resourceType");

return document;
}
Expand Down
13 changes: 12 additions & 1 deletion packages/typespec-azure-core/lib/models.tsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,17 @@ scalar eTag extends string;
*/
scalar azureLocation extends string;

/**
* Represents an Azure Resource Type.
*
* @example
*
* ```
* Microsoft.Network/virtualNetworks/subnets
* ```
*/
scalar resourceType extends string;
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I was OOF the last 2 weeks, has there been agreement we needed this and this was the name we wanted?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh - I do not know. Do we need to bring up this topic in one of our typespec sync up meeting or your internal typespec sync up meeting?
In those meetings that I have participated in, this has not been a topic there.
Or maybe we could put this as an agenda in our next typespec sync up meeting on your wednesday night.

Copy link
Member

Choose a reason for hiding this comment

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

yeah would be good, do you need this for this release?

Copy link
Member

Choose a reason for hiding this comment

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

I think we probably need to differentiate this as we did with armResourceIdentifier - armResourceType.

There is definitely use for this in ARM, and since the type is needed to properly specify the allowedResourceType in armResourceIdentifier, it seems appropriate to put this here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I agree the name should be armResourceType which makes more sense.
The second part is that you suggest we add a generic argument for allowed resource type? like this?

scalar armResourceType<AllowedResourceTypes extends ArmResourceIdentifierAllowedResource[] = never> extends string {}

I think this looks weird because this type is modeling the resource type of myself, if there is a limited list of resource types that this field is supporting, should we define a extensible enum based on resource type like this?

union SupportedResourceType extends armResourceType {
	"Microsoft.Compute/virtualMachines"
}

Well but I could see values for this because we could update the definition of Resource and TrackedResource to let them accept another argument of resource type like this:

model TrackedResource<Properties extends Model, ResourceType extends ArmResourceIdentifierAllowedResource> {
id: armResourceIdentifier;
type: armResourceType<ResourceType>;

properties: Properties;
}

so that we finally have explicitly defined resource type on resources, currently arm resources could have resource types but they are implicitly - we can infer them from the uri paths we built for resource related operations.

Copy link
Member

Choose a reason for hiding this comment

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

yeah I think using a union here instead to be more precise makes more sense instead of the armREsourceIdentifier which does that for a subset of the value.

and +1 on the name being armResourceType


/**
* A type definition that refers the id to an Azure Resource Manager resource.
*
Expand Down Expand Up @@ -398,7 +409,7 @@ alias AllArmResourceDeploymentScopes = [

model ArmResourceIdentifierAllowedResource {
/** The type of resource that is being referred to. For example Microsoft.Network/virtualNetworks or Microsoft.Network/virtualNetworks/subnets. See Example Types for more examples. */
type: string;
type: resourceType;

/**
* An array of scopes. If not specified, the default scope is ["ResourceGroup"].
Expand Down
2 changes: 1 addition & 1 deletion packages/typespec-azure-resource-manager/lib/common-types/types.tsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ model Resource {

/** The type of the resource. E.g. "Microsoft.Compute/virtualMachines" or "Microsoft.Storage/storageAccounts" */
@visibility("read")
type?: string;
type?: resourceType;

/** Azure Resource Manager metadata containing createdBy and modifiedBy information. */
@visibility("read")
Expand Down
Loading