Skip to content

[Typespec Migration] migrate control plane containerregistry to typespec #37169

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
Merged
pshao25 merged 48 commits into from
Oct 27, 2025
Merged

[Typespec Migration] migrate control plane containerregistry to typespec #37169

pshao25 merged 48 commits into from
Oct 27, 2025

Conversation

@kazrael2119
Copy link
Member

@kazrael2119 kazrael2119 commented Sep 5, 2025 •
edited
Loading

This PR migrates your latest version (identified by the tag in your readme.md) of swagger to TypeSpec. We already tried our best to make sure the TypeSpec represents same as previous swagger. Since we lack the business knowledge, please validate this PR again to make sure it's functional equivalent as before. The local validation step is at Getting started | TypeSpec Azure

Besides, TypeSpec encourages to follow ARM guidelines. Therefore, some representations in your previous swagger will be fixed to follow these guidelines. When you see differences in your local validation, please keep this note in mind.

Please reach out to TypeSpec Discussions Channel if there is any help needed.

@github-actions
Copy link

github-actions bot commented Sep 5, 2025 •
edited
Loading

Next Steps to Merge

✅ All automated merging requirements have been met! To get your PR merged, see aka.ms/azsdk/specreview/merge.

Comment generated by summarize-checks workflow run.

@github-actions
Copy link

github-actions bot commented Sep 5, 2025 •
edited
Loading

API Change Check

APIView identified API level changes in this PR and created the following API reviews

Language API Review for Package
Swagger Microsoft.ContainerRegistry-Registry
Swagger Microsoft.ContainerRegistry-RegistryTasks
TypeSpec Microsoft.ContainerRegistry
Go sdk/resourcemanager/containerregistry/armcontainerregistrytasks
JavaScript @azure/arm-containerregistry
JavaScript @azure/arm-containerregistrytasks
Java com.azure.resourcemanager:azure-resourcemanager-containerregistry-generated
Java com.azure.resourcemanager:azure-resourcemanager-containerregistrytasks
Go sdk/resourcemanager/containerregistry/armcontainerregistry
Python azure-mgmt-containerregistry
Python azure-mgmt-containerregistrytasks

@github-actions github-actions bot added the VersioningReviewRequired <valid label in PR review process>add this label when versioning review is required label Sep 5, 2025
@kazrael2119 kazrael2119 changed the title containerregistry conversion [Typespec Migration] migrate containerregistry to typespec Sep 9, 2025
@qiaozha qiaozha changed the title [Typespec Migration] migrate containerregistry to typespec [Typespec Migration] migrate control plane containerregistry to typespec Sep 11, 2025
@tadelesh tadelesh mentioned this pull request Oct 17, 2025
Open
11 tasks
This was referenced Oct 23, 2025
Closed
Closed
jliusan
jliusan reviewed Oct 23, 2025
View reviewed changes
...tainerregistry/resource-manager/Microsoft.ContainerRegistry/Registry/PrivateLinkResource.tsp Outdated
namespace Microsoft.ContainerRegistry;

model MyPrivateLinkResource is PrivateLink;
alias PrivateLinkOperations = PrivateLinks<MyPrivateLinkResource>;
Copy link
Member

@jliusan jliusan Oct 23, 2025

Choose a reason for hiding this comment

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

Does this change by designed ？

Copy link
Member Author

@kazrael2119 kazrael2119 Oct 23, 2025

Choose a reason for hiding this comment

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

I referred to the test case in this PR to modify the typespec code to support arm PrivateLinkResource template for this service

Copy link
Member

@tadelesh tadelesh Oct 23, 2025

Choose a reason for hiding this comment

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

But the MyPRivateLinkResource is not the defined name in swagger.

Copy link
Member Author

@kazrael2119 kazrael2119 Oct 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

this would not affect swagger and sdk generation.
Do you think it is better to use

alias PrivateLinkOperations = PrivateLinks<PrivateLink>;

directly to reduce confusion?

Copy link
Member

@tadelesh tadelesh Oct 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It affected Go SDK generation. We generated the resource with MyPRivateLinkResource name. Could you reach out to @pshao25 to see how to express it.

Copy link
Member Author

@kazrael2119 kazrael2119 Oct 27, 2025

Choose a reason for hiding this comment

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

I have updated the code in ee1656a#diff-5a63799561fd0b3b14e61e5e0f6de5cf19217505b23b1bf769babae5a60de2f4
could you help check again?

Copy link
Member

@tadelesh tadelesh Oct 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

We have tested the converted version. The resource model name has been changed from PrivateLinkResource to PrivateLink which is a break for Go.

Copy link
Member Author

@kazrael2119 kazrael2119 Oct 27, 2025

Choose a reason for hiding this comment

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

update to PrivateLinks<PrivateLinkResource>

@pshao25 pshao25 added BreakingChange-Approved-Benign Changes are not breaking at the REST API level and have at most minor impact to generated SDKs. Approved-LintDiff labels Oct 27, 2025
@github-actions github-actions bot added WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required and removed NotReadyForARMReview labels Oct 27, 2025
@pshao25 pshao25 added the PublishToCustomers Acknowledgement the changes will be published to Azure customers. label Oct 27, 2025
@pshao25 pshao25 added the ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review label Oct 27, 2025
@github-actions github-actions bot removed the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Oct 27, 2025
@pshao25 pshao25 merged commit 6fb4ee6 into Azure:main Oct 27, 2025
56 of 59 checks passed
tadelesh
tadelesh reviewed Oct 27, 2025
View reviewed changes
...ification/containerregistry/resource-manager/Microsoft.ContainerRegistry/Registry/models.tsp
Comment on lines +1611 to +1613
enum ContainerRegistryResourceType {
`Microsoft.ContainerRegistry/registries`,
}
Copy link
Member

@tadelesh tadelesh Oct 27, 2025

Choose a reason for hiding this comment

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

this should be a constant.

@kazrael2119 kazrael2119 deleted the containerregistry-conversion branch October 28, 2025 02:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tadelesh tadelesh tadelesh left review comments

@jliusan jliusan jliusan left review comments

@CodeJourneys CodeJourneys CodeJourneys left review comments

@AntonioVT AntonioVT AntonioVT left review comments

@pshao25 pshao25 pshao25 approved these changes

Assignees

No one assigned

Labels

Approved-LintDiff ARMReview ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review BreakingChange-Approved-Benign Changes are not breaking at the REST API level and have at most minor impact to generated SDKs. BreakingChange-Go-Sdk BreakingChange-Go-Sdk-Approved BreakingChange-JavaScript-Sdk BreakingChange-JavaScript-Sdk-Approved BreakingChange-Python-Sdk BreakingChange-Python-Sdk-Approved BreakingChangeReviewRequired <valid label in PR review process>add this label when breaking change review is required Container Registry PublishToCustomers Acknowledgement the changes will be published to Azure customers. resource-manager TypeSpec Authored with TypeSpec

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.