Skip to content

Clean up typespec issues in ACR #39031

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

Merged
ArcturusZhang merged 5 commits into from
Dec 15, 2025
Merged

Clean up typespec issues in ACR #39031

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8d57e24
fix some existing issues
ArcturusZhang Dec 2, 2025
1daf2dc
more updates
ArcturusZhang Dec 2, 2025
a12fd5e
more updates
ArcturusZhang Dec 2, 2025
575361b
revert the client changes
ArcturusZhang Dec 2, 2025
17bfbae
format
ArcturusZhang Dec 2, 2025
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
60 changes: 7 additions & 53 deletions ...ation/containerregistry/resource-manager/Microsoft.ContainerRegistry/Registry/Archive.tsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,16 @@ using TypeSpec.Rest;
using Azure.ResourceManager;
using TypeSpec.Http;
using TypeSpec.Versioning;
using TypeSpec.OpenAPI;

namespace Microsoft.ContainerRegistry;

// we must have this legacy operations right now because there is a virtual resource `Package` between `Achieve` and `Registry`
// but we did not decide about how to represent such scenarios with "virtual resources" therefore we leave this legacy operations here for now.
#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-interface-requires-decorator" "FIXME: Update justification, follow aka.ms/tsp/conversion-fix for details"
interface AchieveOperationOps
extends Azure.ResourceManager.Legacy.LegacyOperations<
{
...ApiVersionParameter,
...SubscriptionIdParameter,
...ResourceGroupParameter,
...Azure.ResourceManager.Legacy.Provider,

/**
* The name of the container registry.
*/
@path
@segment("registries")
@pattern("^[a-zA-Z0-9]*$")
@minLength(5)
@maxLength(50)
@doc("The name of the container registry.")
registryName: string,
Comment on lines -20 to -34
Copy link
Member Author

@ArcturusZhang ArcturusZhang Dec 2, 2025

Choose a reason for hiding this comment

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

I changed this to simplify. They exactly the same.

...ResourceInstanceParameters<Registry>,

/**
* The package type.
Expand All @@ -58,40 +45,6 @@ interface AchieveOperationOps
}
> {}

#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-interface-requires-decorator" "FIXME: Update justification, follow aka.ms/tsp/conversion-fix for details"
interface AchieveOps
extends Azure.ResourceManager.Legacy.LegacyOperations<
{
...ApiVersionParameter,
...SubscriptionIdParameter,
...ResourceGroupParameter,
...Azure.ResourceManager.Legacy.Provider,

/**
* The name of the container registry.
*/
@path
@segment("registries")
@pattern("^[a-zA-Z0-9]*$")
@minLength(5)
@maxLength(50)
@doc("The name of the container registry.")
registryName: string,

/**
* The package type.
*/
@path
@minLength(3)
@maxLength(50)
@pattern("^[a-zA-Z]*$")
@segment("packages")
@doc("The package type.")
packageType: string,
},
{}
> {}
Comment on lines -61 to -93
Copy link
Member Author

@ArcturusZhang ArcturusZhang Dec 2, 2025

Choose a reason for hiding this comment

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

this is no longer used, because the operation that previously used this should not be using this in the first place.


/**
* An object that represents a archive for a container registry.
*/
Expand Down Expand Up @@ -137,9 +90,10 @@ interface Archives {
/**
* Lists all archives for the specified container registry and package type.
*/
@action("archives")
@list
list is AchieveOps.Read<Archive, Response = ArmResponse<ArchiveListResult>>;
list is AchieveOperationOps.List<
Archive,
Response = ArmResponse<ArchiveListResult>
>;
Comment on lines +93 to +96
Copy link
Member Author

@ArcturusZhang ArcturusZhang Dec 2, 2025

Choose a reason for hiding this comment

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

two issues:
this is a LIST operation of an ARM resource (list itself by its parent), therefore we should use List template instead of Read template.
Read template always represents to the GET method of an ARM resource, it does not represent "any arbitrary method with the verb GET". For instance List is also Get.

}

@@maxLength(Archive.name, 200);
Expand Down
66 changes: 3 additions & 63 deletions ...ontainerregistry/resource-manager/Microsoft.ContainerRegistry/Registry/ArchiveVersion.tsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,15 @@ using TypeSpec.Rest;
using Azure.ResourceManager;
using TypeSpec.Http;
using TypeSpec.Versioning;
using TypeSpec.OpenAPI;

namespace Microsoft.ContainerRegistry;

// we must have this legacy operations right now because there is a virtual resource `Package` between `Achieve` and `Registry`
#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-interface-requires-decorator" "FIXME: Update justification, follow aka.ms/tsp/conversion-fix for details"
interface ArchiveVersionOperationOps
extends Azure.ResourceManager.Legacy.LegacyOperations<
{
...ApiVersionParameter,
...SubscriptionIdParameter,
...ResourceGroupParameter,
...Azure.ResourceManager.Legacy.Provider,

/**
* The name of the container registry.
*/
@path
@segment("registries")
@pattern("^[a-zA-Z0-9]*$")
@minLength(5)
@maxLength(50)
@doc("The name of the container registry.")
registryName: string,
Comment on lines -20 to -34
Copy link
Member Author

@ArcturusZhang ArcturusZhang Dec 2, 2025

Choose a reason for hiding this comment

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

same reason, simplification

...ResourceInstanceParameters<Registry>,

/**
* The package type.
Expand Down Expand Up @@ -69,50 +55,6 @@ interface ArchiveVersionOperationOps
}
> {}

#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-interface-requires-decorator" "FIXME: Update justification, follow aka.ms/tsp/conversion-fix for details"
interface ArchiveVersionListOps
extends Azure.ResourceManager.Legacy.LegacyOperations<
{
...ApiVersionParameter,
...SubscriptionIdParameter,
...ResourceGroupParameter,
...Azure.ResourceManager.Legacy.Provider,

/**
* The name of the container registry.
*/
@path
@segment("registries")
@pattern("^[a-zA-Z0-9]*$")
@minLength(5)
@maxLength(50)
@doc("The name of the container registry.")
registryName: string,

/**
* The package type.
*/
@path
@segment("packages")
@minLength(3)
@maxLength(50)
@pattern("^[a-zA-Z]*$")
@doc("The package type.")
packageType: string,

/**
* The name of the archive resource.
*/
@path
@segment("archives")
@pattern("^[a-zA-Z0-9-]*$")
@minLength(5)
@maxLength(200)
@doc("The name of the archive resource.")
archiveName: string,
},
{}
> {}
Comment on lines -72 to -115
Copy link
Member Author

@ArcturusZhang ArcturusZhang Dec 2, 2025

Choose a reason for hiding this comment

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

this is no longer used.

/**
* An object that represents an export pipeline for a container registry.
*/
Expand Down Expand Up @@ -152,9 +94,7 @@ interface ArchiveVersions {
/**
* Lists all archive versions for the specified container registry, repository type and archive name.
*/
@action("versions")
@list
list is ArchiveVersionListOps.Read<
list is ArchiveVersionOperationOps.List<
Copy link
Member Author

@ArcturusZhang ArcturusZhang Dec 2, 2025

Choose a reason for hiding this comment

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

change to fix its purpose - this is a list, not action or get.

ArchiveVersion,
Response = ArmResponse<ArchiveVersionListResult>
>;
Expand Down
23 changes: 2 additions & 21 deletions ...fication/containerregistry/resource-manager/Microsoft.ContainerRegistry/Registry/main.tsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,6 @@ import "./PipelineRun.tsp";
// Webhook feature
import "./Webhook.tsp";

using TypeSpec.Rest;
using TypeSpec.Http;
using Azure.ResourceManager.Foundations;
using Azure.Core;
using Azure.ResourceManager;
using TypeSpec.Versioning;
Expand Down Expand Up @@ -83,21 +80,5 @@ enum Versions {
vLatestPreview: "2025-06-01-preview",
}

interface Operations {
#suppress "@azure-tools/typespec-azure-core/no-private-usage" "FIXME: Update justification, follow aka.ms/tsp/conversion-fix for details"
#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-operation" "FIXME: Update justification, follow aka.ms/tsp/conversion-fix for details"
@tag("Operations")
@autoRoute
@Azure.ResourceManager.Private.armUpdateProviderNamespace
@doc("List the operations for the provider")
@segment("operations")
@list
list(
...ApiVersionParameter,

@path
@segment("providers")
@doc("The provider namespace (this parameter will not show up in operations).")
provider: "Microsoft.ThisWillBeReplaced",
): ArmResponse<OperationListResult> | ErrorResponse;
}
interface Operations
extends Azure.ResourceManager.Legacy.Operations<Response = ArmResponse<OperationListResult>> {}
112 changes: 0 additions & 112 deletions ...cation/containerregistry/resource-manager/Microsoft.ContainerRegistry/Registry/models.tsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,7 @@ import "@azure-tools/typespec-azure-core";
import "@azure-tools/typespec-azure-resource-manager";
import "@typespec/versioning";

using TypeSpec.Rest;
using TypeSpec.Http;
using TypeSpec.Versioning;
using Azure.Core;
using Azure.ResourceManager;
using Azure.ResourceManager.Foundations;
namespace Microsoft.ContainerRegistry;

/**
Expand Down Expand Up @@ -83,72 +78,6 @@ enum ResourceIdentityType {
None,
}

/**
* The resource model definition for a ARM proxy resource. It will have everything other than required location and tags.
*/
model ProxyResource {
/**
* The resource ID.
*/
@visibility(Lifecycle.Read)
id?: string;

/**
* The name of the resource.
*/
@visibility(Lifecycle.Read)
name?: string;

/**
* The type of the resource.
*/
@visibility(Lifecycle.Read)
type?: string;

/**
* Metadata pertaining to creation and last modification of the resource.
*/
@visibility(Lifecycle.Read)
systemData?: SystemData;
}

/**
* Metadata pertaining to creation and last modification of the resource.
*/
model SystemData {
/**
* The identity that created the resource.
*/
createdBy?: string;

/**
* The type of identity that created the resource.
*/
createdByType?: CreatedByType;

/**
* The timestamp of resource creation (UTC).
*/
// FIXME: (utcDateTime) Please double check that this is the correct type for your scenario.
createdAt?: utcDateTime;

/**
* The identity that last modified the resource.
*/
lastModifiedBy?: string;

/**
* The type of identity that last modified the resource.
*/
lastModifiedByType?: LastModifiedByType;

/**
* The timestamp of resource modification (UTC).
*/
// FIXME: (utcDateTime) Please double check that this is the correct type for your scenario.
lastModifiedAt?: utcDateTime;
}

/**
* Managed identity for the resource.
*/
Expand Down Expand Up @@ -352,47 +281,6 @@ model OperationListResult {
nextLink?: string;
}

/**
* An Azure resource.
*/
model Resource {
/**
* The resource ID.
*/
@visibility(Lifecycle.Read)
id?: string;

/**
* The name of the resource.
*/
@visibility(Lifecycle.Read)
name?: string;

/**
* The type of the resource.
*/
@visibility(Lifecycle.Read)
type?: string;

/**
* The location of the resource. This cannot be changed after the resource is created.
*/
@visibility(Lifecycle.Read, Lifecycle.Create)
location: string;

/**
* The tags of the resource.
*/
#suppress "@azure-tools/typespec-azure-resource-manager/arm-no-record" "FIXME: Update justification, follow aka.ms/tsp/conversion-fix for details"
tags?: Record<string>;

/**
* Metadata pertaining to creation and last modification of the resource.
*/
@visibility(Lifecycle.Read)
systemData?: SystemData;
}

/**
* The Active Directory Object that will be used for authenticating the token of a container registry.
*/
Expand Down
8 changes: 3 additions & 5 deletions ...on/containerregistry/resource-manager/Microsoft.ContainerRegistry/Registry/tspconfig.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,10 @@ options:

arm-types-dir: "{project-root}/../../../../common-types/resource-management"
emit-lro-options: "all"
"@azure-tools/typespec-csharp":
"@azure-typespec/http-client-csharp-mgmt":
emitter-output-dir: "{output-dir}/{service-dir}/{namespace}"
flavor: azure
clear-output-folder: true
model-namespace: true
namespace: "Azure.ResourceManager.ContainerRegistry"
package-name: "Azure.ResourceManager.ContainerRegistry"
namespace: "{package-name}"
"@azure-tools/typespec-python":
service-dir: "sdk/containerregistry"
emitter-output-dir: "{output-dir}/{service-dir}/azure-mgmt-containerregistry"
Expand Down
Loading