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

resource model changes of generated SDK code based on TSP compared with swagger #104

Closed
Tracked by #72
msyyc opened this issue Jan 10, 2024 · 7 comments
Closed
Tracked by #72
Assignees
Milestone

Comments

@msyyc
Copy link
Member

msyyc commented Jan 10, 2024

By comparing diff about generated SDK between TSP and swagger, I find some model name changes:

  • Resource-> ArmResource
  • ProxyResource-> ProxyResourceBase
  • TrackedResource-> TrackedResourceBase

I think it is related with tsp definition of tyepsec-azure-resource-manager

@allenjzhang
Copy link
Member

Is this the exhaust list? @@projectedName should be used to adjust client name.

@tadelesh
Copy link
Member

Workaround: review all TSP ARM model and rename them with @projectedName according to the name in common types.

@markcowl
Copy link
Member

markcowl commented Jan 11, 2024

If we are going to support very old APIs, there are several possible patterns:

  • Resource is defined in the spec
  • Resource types inherit from 'Resource' or similar common schema
  • Resource types inherit from another common schema (e.g. ProxyResource, even if they are tracked resources)

If client generation is sensitive to inheritance from common-types, thenthe right things likely is to generate the proper inheritance in this case and not use the resource templates, the template constraints use structural typing, not nominal typing, so any model with the right properties will work as in this playground

@tadelesh
Copy link
Member

tadelesh commented Jan 16, 2024

@markcowl thanks for your solution for non-standard resource definition.

Two concerns for the standard resource:

  1. When sdk emitter get models info directly from tsp, we will get ProxyResourceBase as the base model, which name is different from swagger definition, thus will cause breaking changes for brown field RPs.
  2. I don't see different version's common types defined in arm lib. Could sdk emitter get different properties info according to @armCommonTypesVersion setting directly from tsp compile?

@markcowl
Copy link
Member

@tadelesh Those types also have an @armCommonTypes definition - the generator should be making these decisions for compliant specs by the ARM common types definition, so this decorator may be useful in the 'compliant' case.

There are differences in the common types version, strictly around type formats (resourceId, uri, uuid), there have not, to date been any substantial differences other than these

@markcowl
Copy link
Member

The solution we have discussed is:

  • Rename the types in the inheritance chain for ProxyResource, TrackedResource and ExtensionResource, so these align with common type schema names
  • Add models matching other resource types in common schema for use when defining non-standard (not ProxyResource or Trackedresource) inheritance.

@markcowl markcowl added this to the [2024] May milestone May 7, 2024
@allenjzhang
Copy link
Member

#762

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants