2022-03-03 Compute/GalleryRP StorageAccountType Swagger failing

[JenGoldstrich]

👋 Hello,

In the 2022-03-03 Compute Gallery API the storageAccountType enum is defined with a different set of allowed values

azure-rest-api-specs/specification/compute/resource-manager/Microsoft.Compute/GalleryRP/stable/2022-03-03/gallery.json

Line 2469 in 076362b

"storageAccountType": {

https://github.com/Azure/azure-rest-api-specs/blob/main/specification/compute/resource-manager/Microsoft.Compute/GalleryRP/stable/2022-03-03/gallery.json#L2560

This is causing an error for me when I am trying to add the 2022-03-03 compute APi to the SDK I am working on because of conflicting constants

error: -12T15:15:26.126Z [ERROR] Importer for Service "Compute".Importer for API Version "2022-03-03":      💥 Error: parsing Swagger files: parsing files in "../../swagger/specification/compute/resource-manager/Microsoft.Compute/GalleryRP/stable/2022-03-03": parsing definition: finding resources for tag "GalleryApplicationVersions": finding nested items yet to be parsed: appending model: appending constants: conflicting constant "StorageAccountType" with different values. First: map[PremiumLRS:Premium_LRS StandardLRS:Standard_LRS StandardZRS:Standard_ZRS]. Second: map[PremiumLRS:Premium_LRS StandardLRS:Standard_LRS StandardSSDLRS:StandardSSD_LRS StandardZRS:Standard_ZRS]

This seems to have been introduced in #22255, looking at this PR I see the same change was also applied to the two previous API versions (2021-10-01 and 2022-01-03)

I see another PR here #22829 which was opened to address this which seems like a logical solution but am unsure why it was closed

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @TravisCragg-MSFT, @sandeepraichura.

Issue Details

---

👋 Hello,

In the 2022-03-03 Compute Gallery API the storageAccountType enum is defined with a different set of allowed values

azure-rest-api-specs/specification/compute/resource-manager/Microsoft.Compute/GalleryRP/stable/2022-03-03/gallery.json

Line 2469 in 076362b

"storageAccountType": {

https://github.com/Azure/azure-rest-api-specs/blob/main/specification/compute/resource-manager/Microsoft.Compute/GalleryRP/stable/2022-03-03/gallery.json#L2560

This is causing an error for me when I am trying to add the 2022-03-03 compute APi to the SDK I am working on because of conflicting constants

error: -12T15:15:26.126Z [ERROR] Importer for Service "Compute".Importer for API Version "2022-03-03":      💥 Error: parsing Swagger files: parsing files in "../../swagger/specification/compute/resource-manager/Microsoft.Compute/GalleryRP/stable/2022-03-03": parsing definition: finding resources for tag "GalleryApplicationVersions": finding nested items yet to be parsed: appending model: appending constants: conflicting constant "StorageAccountType" with different values. First: map[PremiumLRS:Premium_LRS StandardLRS:Standard_LRS StandardZRS:Standard_ZRS]. Second: map[PremiumLRS:Premium_LRS StandardLRS:Standard_LRS StandardSSDLRS:StandardSSD_LRS StandardZRS:Standard_ZRS]

This seems to have been introduced in #22255, looking at this PR I see the same change was also applied to the two previous API versions (2021-10-01 and 2022-01-03)

I see another PR here #22829 which was opened to address this which seems like a logical solution but am unsure why it was closed

Author:	JenGoldstrich
Assignees:	-
Labels:	Compute - Images, Service Attention
Milestone:	-

[Drewm3]

@sandeepraichura, could you please look into this issue.

[sandeepraichura]

Thanks for reporting it, we are currently looking into the issue and I'll keep you posted on the status in a couple of weeks.

[ArcturusZhang]

Hi @JenGoldstrich it is unfortunate that you get this issue because of the divergence of the enum values - we once had this issue when generating the "track 1" SDK which is an old version of the SDK generated from an old version generator and now it is deprecated.
The way we fix that is to use an additional patch layer of the tool to dynamically patch the generator to give one of the enums to have a different name:

azure-rest-api-specs/specification/compute/resource-manager/readme.csharp.md

Line 39 in 38311a1

transform: return "EdgeZoneStorageAccountType"

In the new version of the SDKs, the generator could properly handle this case without issues by combining those two different enums. Side effect is that one enum has an additional value which is not valid right now. But this is coming from the thought that if those two enums have the same name, they are very probably to be the same thing, despite they diverged now, there is possibility that in the future, they no longer diverge.

We definitely do not want to just change it on the swagger because that would be a breaking change to all of our current SDKs, therefore I would like to ask if there could be any way to work around on your tool to properly handle cases like this?

[tombuildsstuff]

hey @ArcturusZhang

Whilst we do have the ability to do temporary workarounds within our importer - we intentionally only do these whilst the upstream data is being fixed (meaning they can be removed soon after) - so that's not something we can do as a long-term workaround without fixing the source data here unfortunately.

I'd actually argue the behaviour used by Track2 here is probably a bug, rather than a feature - since if multiple Enums are defined with the same name and differing values, how are users supposed to know which value is valid for which field? Instead, I'd suggest that these should be being caught by the OpenAPI Validator/Linting, since whilst I can understand it's possible for an Enum to be unintentionally used in the manner you're describing - as a consumer of that data, it's far more useful to have a missing Enum value (which can easily be added, since it's a non-breaking change) - verses being unable to determine which Enum values are valid. cc @mikekistler / @JeffreyRichter

Whilst obviously the semantics of that are larger than this specific issue, is it possible to get the source data fixed here and add an Autorest workaround for Track2 SDK's, so that the Swagger is correct here - and then look to determine if this behaviour should be updated?

Thanks!

[JeffreyRichter]

I tried reading this thread and I got lost. Can someone explain the whole thing to me in detail and then I can weigh in with an opinion as to how to fix/improve this.

[grizzlytheodore]

@JeffreyRichter there are several places that define enum values for: storageAccountType
but one of them has more enum values than the other, so SDK generation is failing for some.

In this PR: #23741 we are updating the one enum name to differentiate them