[Network] [NRP] api-version inside couple of spec files does not match the folder they are in

Azure / azure-rest-api-specs

The source for REST API specifications for Microsoft Azure.

MIT License

2.68k stars 5.1k forks source link

[Network] [NRP] api-version inside couple of spec files does not match the folder they are in #1685

Open veronicagg opened 7 years ago

veronicagg commented 7 years ago

Api-version in the spec file, does not match the folder it's in: https://github.com/Azure/azure-rest-api-specs/blob/current/specification/network/resource-manager/Microsoft.Network/2017-09-01/vmssNetworkInterface.json#L6 https://github.com/Azure/azure-rest-api-specs/blob/current/specification/network/resource-manager/Microsoft.Network/2017-09-01/vmssPublicIpAddress.json#L6

This is happening not only in the latest api version but previous one too. Please check and if the apis exist for corresponding version the value of "version" should be updated in the specs.

lmazuel commented 7 years ago

@veronicagg This is a long story.... VMSS depends on Microsoft.Compute, this means that its APIVersion follows Compute. However, this is Network information, and to be able to share models from regular NIC to VMSS NIC, a long time ago they decided to put VMSS in the Network package.

This is a cause of trouble for a long time (@olydis @johanste). And I'm not even talking about AzureStack support.... We decided at the Python level to ship these files part of the Compute package. This should be part of the next Compute release.

veronicagg commented 7 years ago

@lmazuel for code generation purposes, are you creating a tag for a package in network which does not contain these 2 specs? at https://github.com/Azure/azure-rest-api-specs/tree/current/specification/network/resource-manager? I don't see these specs in Compute folder. Should these specs be at a Microsoft.Compute folder inside https://github.com/Azure/azure-rest-api-specs/tree/current/specification/network/resource-manager ? How are you going to ship them in Compute? Since we're starting to split code generation by api-version, these 2 files don't go well together with the rest of the files in network packages.

lmazuel commented 7 years ago

There is a hack if you didn't see, look into these files you will see they truly belong to "Microsoft.Compute". The folder name is a lie :).

These two specs have references to NIC definition. Move them to "compute" has different issues:

Solution 1: copy NIC definitions with the files. What version of NIC definition do we copy? Who owns the mapping between the correct NIC definition and the correct VMSS definition?
Solution 2: keep referencing it from Compute, like "../../../../network/resource-manager/Microsoft.Network/2017-09-01/networkInterface.json". Ugly as possible....

For generation, this indeed needs a new tag, or adding them as "input-file". I honestly didn't start yet, so I'm not sure if there is caveats I didn't see. But add them as input file should be simple, Autorest will solve the references and add the correct NIC models for free in Compute.

veronicagg commented 7 years ago

@lmazuel I tried referencing them from Compute, as I think that would be best. There are already other files in compute which share the api version, so I attempted generation of all those files together and get a code generation error from AutoRest, because the models particularly for "Resource" conflict (not the same). At the moment, I'm going with our older approach, including them as part of Network, until we figure out a better route.

lmazuel commented 7 years ago

Thanks @veronicagg. That's really unfortunate... :(

MikhailTryakhov commented 6 years ago

fixed

lmazuel commented 6 years ago

@MikhailTryakhov Not fixed, that's why we have a meeting this afternoon :)