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

[Bug]: New model UserAssignedIdentities will cause breaking changes for SDK #824

Closed
4 tasks done
XiaofeiCao opened this issue May 11, 2024 · 3 comments
Closed
4 tasks done
Labels
bug Something isn't working lib:tcgc Issues for @azure-tools/typespec-client-generator-core library needs-area

Comments

@XiaofeiCao
Copy link
Contributor

XiaofeiCao commented May 11, 2024

Describe the bug

In 0.42, tsp-arm seems to introduce a new model UserAssignedIdentities:

model UserAssignedIdentities is Record<UserAssignedIdentity>;

though previously, it's a property whose type is defined as Record:
userAssignedIdentities?: Record<UserAssignedIdentity>;

TCGC will generate a new model UserAssignedIdentities if it's defined is Record, which changes Java's generated ManagedServiceIdentity's userAssignedIdentities property type from Map<String, UserAssignedIdentity> to nowUserAssignedIdentities. This a breaking change. See Azure/autorest.java#2698 (comment)

I wonder why we decide to change the UserAssignedIdenties definition?
BTW, isn't model is Record discouraged in ARM?
https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/no-record#-incorrect-1

Reproduction

It happens for SDK generation. In Swagger, it refers to the common model.

Checklist

  • Follow our Code of Conduct
  • Check that this issue is about the Azure libraries for typespec. For bug in the typespec language or core libraries file it in the TypeSpec repo
  • Check that there isn't already an issue that request the same bug to avoid creating a duplicate.
  • The provided reproduction is a minimal reproducible example of the bug.
@XiaofeiCao XiaofeiCao added bug Something isn't working ARM lib:tcgc Issues for @azure-tools/typespec-client-generator-core library labels May 11, 2024
@XiaofeiCao XiaofeiCao changed the title [Bug]: Change in UserAssignedIdentities definition will cause breaking changes for SDK [Bug]: New model UserAssignedIdentities will cause breaking changes for SDK May 11, 2024
@msyyc
Copy link
Member

msyyc commented May 13, 2024

Python has similar issue, too.

@weidongxu-microsoft
Copy link
Member

weidongxu-microsoft commented May 13, 2024

My guess is that the discourage of Record is for generic usage of additionalProperties.

I think the easiest fix would be as #825 mentioned to revert it to userAssignedIdentities?: Record<UserAssignedIdentity>.

@XiaofeiCao
Copy link
Contributor Author

XiaofeiCao commented May 14, 2024

duplicate, discussion in #825

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lib:tcgc Issues for @azure-tools/typespec-client-generator-core library needs-area
Projects
None yet
Development

No branches or pull requests

3 participants