search

Azure / typespec-azure

About TypeSpec Azure Libraries
https://azure.github.io/typespec-azure/
MIT License
11 stars 37 forks source link

[Bug]: ExtendedLocation is defined as all its properties to be required #1252

Open ArcturusZhang opened 1 month ago

ArcturusZhang commented 1 month ago

Describe the bug

So comparing from the typespec definition here: https://github.com/Azure/typespec-azure/blob/main/packages/typespec-azure-resource-manager/lib/common-types/extended-location.tsp and the swagger definition here: https://github.com/Azure/azure-rest-api-specs/blob/a651ba25cda4eec698a3a4e35f867ecc2681d126/specification/resources/resource-manager/Microsoft.Resources/stable/2024-03-01/resources.json#L5582 The definition in swagger has all its properties optional - but the typespec definition says required.

Reproduction

Check the files above.

Suggestions

Because dotnet SDK already generates some of the models from ARM as common models, something cannot be changed. I think maybe we could change the two properties in the typespec definition to optional?

Checklist

ArcturusZhang commented 1 month ago

We notice this issue because monitor is partially migrating to typespec, and we saw the ExtendedLocation type was generated in the generated code. And the root cause is that they write their own definition of ExtendedLocation, with both properties required. But when we check the common types definition in azure-resourcemanager, we also saw both properties required This is inconsistent with the swagger definition.

allenjzhang commented 1 month ago

This is the ExtendedLocation specification. TypeSpec definition follows that.

ArcturusZhang commented 1 month ago

This is the ExtendedLocation specification. TypeSpec definition follows that.

This would be unfortunate because the swagger definition I shared in the description does not follow this even if the spec belongs to ARM. I do not think it is quite a big deal for dotnet, but it might be breaking for other languages.