Skip to content
This repository has been archived by the owner on Jul 14, 2023. It is now read-only.

Allow option to generate parameter names based on their json body path #868

Open
hivyas opened this issue Jun 29, 2021 · 0 comments
Open

Comments

@hivyas
Copy link
Member

hivyas commented Jun 29, 2021

Current behavior

This is how our video analyzer entity is defined:

"parameters": {
    "subscriptionId": "00000000-0000-0000-0000-000000000000",
    "resourceGroupName": "contoso",
    "accountName": "contosotv",
    "api-version": "2021-05-01-preview",
    "parameters": {
      "location": "South Central US",
      "tags": {
        "tag1": "value1",
        "tag2": "value2"
      },
      "properties": {
       "encryption": {
            "type": "CustomerKey",
            "keyVaultProperties": {
              "keyIdentifier": "https://keyvault.vault.azure.net/keys/userkeys/0000000000000000000000000000000",
              "currentKeyIdentifier": "https://keyvault.vault.azure.net/keys/userkeys/0000000000000000000000000000000"
            },
            "identity": {
              "userAssignedIdentity": "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/rg/providers/Microsoft.ManagedIdentity/userAssignedIdentities/id2"
            },
            "status": "enabled"
        },
        "storageAccounts": [
          {
            "id": "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/rg/providers/Microsoft.Storage/storageAccounts/storage1",
            "identity": {
              "userAssignedIdentity": "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/rg/providers/Microsoft.ManagedIdentity/userAssignedIdentities/id2"
            }
          }
        ]
      },
      "identity": {
        "type": "UserAssigned",
        "userAssignedIdentities": {
          "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/rg/providers/Microsoft.ManagedIdentity/userAssignedIdentities/id1": {},
          "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/rg/providers/Microsoft.ManagedIdentity/userAssignedIdentities/id2": {},
          "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/rg/providers/Microsoft.ManagedIdentity/userAssignedIdentities/id3": {}
        }
      }
    }
  }

And this is what the generated parameters look like:
image

Issues with current behavior

  • Notice how encryption.type from the json body became --type in the parameters, but identity.type became --video-analyzer-identity-type. Autorest.az seems to be assigning a prefix to a repeat parameter name. This is an issue because what if in the future the ordering of our properties in our swagger changes or another property named type gets added? Then the generated names will be different based on what property comes first. This can cause breaking changes in our users' scripts.
  • Notice how in the parameters we have --user-assigned-identity (under Encryption Identity Arguments), --user-assigned-identities (under Identity Arguments), and user-assigned-identity (under --storage-accounts). The only difference between these parameter names is one is singular and the other is plural. This is very ambiguous and can be confusing to customers.

Proposal for new generation option for parameter names

In order to combat these issues, we would like to propose a generation option that can define parameters using their fully qualified json path. To clarify, we are not asking for this to be the default mode during generation; rather have it be enabled through a flag.

What I mean by fully qualified json path:

  • For the property encryption.type: change --type to --encryption-type.
  • For the property identity.type: change --video-analyzer-identity-type to --identity-type.
  • For the property identity.userAssignedIdentities, change --user-assigned-identities to --identity-user-assigned-identities.

And so on.. this naming structure would apply for every property. This will create more clear names and be less confusing to the user as to which group the parameters belong to. This will also avoid breaking changes if the order of the swagger changes.

It appears the capabilities for this feature does exist since currently if Autorest.Az comes across a repeated property name it will append the entity name (--video-analyzer-identity-type)

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

No branches or pull requests

1 participant