Azure
Align the casing of LifetimeActionsType with the service.
Data Plane (and Control Plane) API - Pull Request
Azure Key Vault's data plane has been using PascalCase for the LifetimeActionsType enum values, while the REST API spec incorrectly specifies the casing to be camelCase. This PR aligns the REST API spec (of both data plane and control plane) with the service.
Note that this is not a breaking change in the service (which has been using PascalCase since GA).
API Info: The Basics
Is this review for (select one):
- [ ] a private preview
- [ ] a public preview
- [x] GA release
Hi, @adarce Thanks for your PR. I am workflow bot for review process. Here are some small tips.
![openapi-workflow-bot[bot] avatar](https://avatars.githubusercontent.com/in/81125?v=4 "Published by a pub.dev verified publisher"){kind=small}
Swagger Validation Report
️❌BreakingChange: 30 Errors, 0 Warnings failed [Detail]
| compared swaggers (via Oad v0.10.4)] | new version | base version |
|---|---|---|
| keys.json | 7.3-preview(a424071) | 7.3-preview(main) |
| keys.json | 7.4-preview.1(a424071) | 7.4-preview.1(main) |
| keys.json | 7.5-preview.1(a424071) | 7.5-preview.1(main) |
| keys.json | 7.3(a424071) | 7.3(main) |
| keys.json | 7.4(a424071) | 7.4(main) |
| keys.json | 2021-04-01-preview(a424071) | 2021-04-01-preview(main) |
| keys.json | 2021-06-01-preview(a424071) | 2021-06-01-preview(main) |
| keys.json | 2021-11-01-preview(a424071) | 2021-11-01-preview(main) |
| keys.json | 2022-02-01-preview(a424071) | 2022-02-01-preview(main) |
| keys.json | 2022-07-01(a424071) | 2022-07-01(main) |
| keys.json | 2022-11-01(a424071) | 2022-11-01(main) |
| keysManagedHsm.json | 2022-11-01(a424071) | 2022-11-01(main) |
| keys.json | 2023-02-01(a424071) | 2023-02-01(main) |
| keysManagedHsm.json | 2023-02-01(a424071) | 2023-02-01(main) |
️️✔️Breaking Change(Cross-Version) succeeded [Detail] [Expand]
There are no breaking changes.
️️✔️CredScan succeeded [Detail] [Expand]
There is no credential detected.
️🔄LintDiff inProgress [Detail]
️️✔️Avocado succeeded [Detail] [Expand]
Validation passes for Avocado.
️️✔️SwaggerAPIView succeeded [Detail] [Expand]
️️✔️TypeSpecAPIView succeeded [Detail] [Expand]
️️✔️ModelValidation succeeded [Detail] [Expand]
Validation passes for ModelValidation.
️️✔️SemanticValidation succeeded [Detail] [Expand]
Validation passes for SemanticValidation.
️️✔️PoliCheck succeeded [Detail] [Expand]
Validation passed for PoliCheck.
️️✔️PrettierCheck succeeded [Detail] [Expand]
Validation passes for PrettierCheck.
️️✔️SpellCheck succeeded [Detail] [Expand]
Validation passes for SpellCheck.
️️✔️Lint(RPaaS) succeeded [Detail] [Expand]
Validation passes for Lint(RPaaS).
️️✔️PR Summary succeeded [Detail] [Expand]
Validation passes for Summary.
️️✔️Automated merging requirements met succeeded [Detail] [Expand]
![openapi-pipeline-app[bot] avatar](https://avatars.githubusercontent.com/in/65541?v=4 "Published by a pub.dev verified publisher"){kind=small}
Swagger Generation Artifacts
️️✔️ApiDocPreview succeeded [Detail] [Expand]
Please click here to preview with your @microsoft account.
️⚠️ azure-sdk-for-net-track2 warning [Detail]
⚠️Warning [Logs]Release - Generate from c06b7c22971d4c71e52f0962728c99aabf70127e. SDK Automation 14.0.0command pwsh ./eng/scripts/Automation-Sdk-Init.ps1 ../azure-sdk-for-net_tmp/initInput.json ../azure-sdk-for-net_tmp/initOutput.json warn specification/keyvault/data-plane/readme.md skipped due to azure-sdk-for-net-track2 not found in swagger-to-sdk command pwsh ./eng/scripts/Invoke-GenerateAndBuildV2.ps1 ../azure-sdk-for-net_tmp/generateInput.json ../azure-sdk-for-net_tmp/generateOutput.json warn No file changes detected after generation
️✔️Azure.ResourceManager.KeyVault [View full logs]info [Changelog]
️⚠️ azure-sdk-for-python-track2 warning [Detail]
⚠️Warning [Logs]Release - Generate from c06b7c22971d4c71e52f0962728c99aabf70127e. SDK Automation 14.0.0command sh scripts/automation_init.sh ../azure-sdk-for-python_tmp/initInput.json ../azure-sdk-for-python_tmp/initOutput.json cmderr [automation_init.sh] WARNING: Skipping azure-nspkg as it is not installed. warn specification/keyvault/data-plane/readme.md skipped due to azure-sdk-for-python-track2 not found in swagger-to-sdk command sh scripts/automation_generate.sh ../azure-sdk-for-python_tmp/generateInput.json ../azure-sdk-for-python_tmp/generateOutput.json cmderr [automation_generate.sh] cmderr [automation_generate.sh] npm notice New minor version of npm available! 9.6.7 -> 9.8.1 cmderr [automation_generate.sh] npm notice Changelog: <https://github.com/npm/cli/releases/tag/v9.8.1> cmderr [automation_generate.sh] npm notice Run `npm install -g [email protected]` to update! cmderr [automation_generate.sh] npm notice
️✔️track2_azure-mgmt-keyvault [View full logs] [Release SDK Changes]info [Changelog] change log generation failed!!!
️⚠️ azure-sdk-for-java warning [Detail]
⚠️Warning [Logs]Release - Generate from c06b7c22971d4c71e52f0962728c99aabf70127e. SDK Automation 14.0.0command ./eng/mgmt/automation/init.sh ../azure-sdk-for-java_tmp/initInput.json ../azure-sdk-for-java_tmp/initOutput.json cmderr [init.sh] [notice] A new release of pip is available: 23.0.1 -> 23.2.1 cmderr [init.sh] [notice] To update, run: pip install --upgrade pip cmderr [init.sh] [notice] A new release of pip is available: 23.0.1 -> 23.2.1 cmderr [init.sh] [notice] To update, run: pip install --upgrade pip warn specification/keyvault/data-plane/readme.md skipped due to azure-sdk-for-java not found in swagger-to-sdk command ./eng/mgmt/automation/generate.py ../azure-sdk-for-java_tmp/generateInput.json ../azure-sdk-for-java_tmp/generateOutput.json
️✔️azure-resourcemanager-keyvault-generated [View full logs] [Release SDK Changes]
️️✔️ azure-sdk-for-go succeeded [Detail] [Expand]
️✔️Succeeded [Logs]Release - Generate from c06b7c22971d4c71e52f0962728c99aabf70127e. SDK Automation 14.0.0command sh ./eng/scripts/automation_init.sh ../../../../../azure-sdk-for-go_tmp/initInput.json ../../../../../azure-sdk-for-go_tmp/initOutput.json warn specification/keyvault/data-plane/readme.md skipped due to azure-sdk-for-go not found in swagger-to-sdk command generator automation-v2 ../../../../../azure-sdk-for-go_tmp/generateInput.json ../../../../../azure-sdk-for-go_tmp/generateOutput.json
️✔️sdk/resourcemanager/keyvault/armkeyvault [View full logs] [Release SDK Changes]info [Changelog] ### Features Added info [Changelog] info [Changelog] - New value `ManagedHsmSKUNameCustomB6` added to enum type `ManagedHsmSKUName` info [Changelog] info [Changelog] Total 0 breaking change(s), 1 additive change(s).
️️✔️ azure-sdk-for-js succeeded [Detail] [Expand]
️✔️Succeeded [Logs]Release - Generate from c06b7c22971d4c71e52f0962728c99aabf70127e. SDK Automation 14.0.0command sh .scripts/automation_init.sh ../azure-sdk-for-js_tmp/initInput.json ../azure-sdk-for-js_tmp/initOutput.json warn File azure-sdk-for-js_tmp/initOutput.json not found to read warn specification/keyvault/data-plane/readme.md skipped due to azure-sdk-for-js not found in swagger-to-sdk command sh .scripts/automation_generate.sh ../azure-sdk-for-js_tmp/generateInput.json ../azure-sdk-for-js_tmp/generateOutput.json cmderr [automation_generate.sh] [ERROR] Cannot generate changelog because the codes of local and npm may be the same.
️✔️@azure/arm-keyvault [View full logs] [Release SDK Changes]info [Changelog] error breakingChangeTracking is enabled, but version or changelogItem is not found in output.
️️✔️ azure-resource-manager-schemas succeeded [Detail] [Expand]
️✔️Succeeded [Logs]Release - Generate from c06b7c22971d4c71e52f0962728c99aabf70127e. Schema Automation 14.0.0command .sdkauto/initScript.sh ../azure-resource-manager-schemas_tmp/initInput.json ../azure-resource-manager-schemas_tmp/initOutput.json warn File azure-resource-manager-schemas_tmp/initOutput.json not found to read warn specification/keyvault/data-plane/readme.md skipped due to azure-resource-manager-schemas not found in swagger-to-sdk command .sdkauto/generateScript.sh ../azure-resource-manager-schemas_tmp/generateInput.json ../azure-resource-manager-schemas_tmp/generateOutput.json
️✔️keyvault [View full logs] [Release Schema Changes]
️❌ azure-powershell failed [Detail]
❌Pipeline Framework Failed [Logs]Release - Generate from c06b7c22971d4c71e52f0962728c99aabf70127e. SDK Automation 14.0.0command sh ./tools/SwaggerCI/init.sh ../azure-powershell_tmp/initInput.json ../azure-powershell_tmp/initOutput.json warn specification/keyvault/data-plane/readme.md skipped due to azure-powershell not found in swagger-to-sdk command pwsh ./tools/SwaggerCI/psci.ps1 ../azure-powershell_tmp/generateInput.json ../azure-powershell_tmp/generateOutput.json SSL error: syscall failure: Broken pipe Error: SSL error: syscall failure: Broken pipe
⚠️Az.keyvault.DefaultTag [View full logs]error Fatal error: SSL error: syscall failure: Broken pipe error The following packages are still pending: error Az.keyvault.DefaultTag
Generated ApiView
| Language | Package Name | ApiView Link |
|---|---|---|
| Go | sdk/resourcemanager/keyvault/armkeyvault | https://apiview.dev/Assemblies/Review/8ab3940281cc4abe8689ecfe070548e5 |
| Java | azure-resourcemanager-keyvault-generated | https://apiview.dev/Assemblies/Review/d1662e9c0f0943eeac4ce02129962c0e |
| .Net | Azure.ResourceManager.KeyVault | There is no API change compared with the previous version |
| JavaScript | @azure/arm-keyvault | https://apiview.dev/Assemblies/Review/1f366ad839ac492092365403696ddbf0 |
Hi @adarce, one or multiple breaking change(s) is detected in your PR. Please check out the breaking change(s), and provide business justification in the PR comment and @ PR assignee why you must have these change(s), and how external customer impact can be mitigated. Please ensure to follow breaking change policy to request breaking change review and approval before proceeding swagger PR review. Action: To initiate an evaluation of the breaking change, create a new intake using the template for breaking changes. Addition details on the process and office hours are on the Breaking change Wiki. If you want to know the production traffic statistic, please see ARM Traffic statistic. If you think it is false positive breaking change, please provide the reasons in the PR comment, report to Swagger Tooling Team via https://aka.ms/swaggerfeedback. Note: To avoid breaking change, you can refer to Shift Left Solution for detecting breaking change in early phase at your service code repository.
When will the service roll out?
The change to revert the service back to using "Notify"/"Rotate" instead of lowercase has been merged. The next deployment will probably begin early next week, and it takes 5 business days to complete.
@JeffreyRichter @mikekistler this is aligning the swagger to match the service - or at least what the service did do and will soon again. Can you approve?
Lets Add CP to it, so we are consistent.
@jlichwa is that actually what CP returns though? In a previous conversation, you indicated that DP was likely changed to align with CP.
Lets Add CP to it, so we are consistent.
@jlichwa is that actually what CP returns though? In a previous conversation, you indicated that DP was likely changed to align with CP.
I checked CP has lower case. We aligned service to Swagger specs which all had lower case, as we now change service back to PascalCase- I want it to be consistent everywhere, so swagger needs to be updated as well as service change rolled back.
I believe @adarce created separate PR for CP
Lets Add CP to it, so we are consistent.
@jlichwa is that actually what CP returns though? In a previous conversation, you indicated that DP was likely changed to align with CP.
I checked CP has lower case. We aligned service to Swagger specs which all had lower case, as we now change service back to PascalCase- I want it to be consistent everywhere, so swagger needs to be updated as well as service change rolled back.
I believe @adarce created separate PR for CP
CP's implementation does not attempt to perform any normalization for casing.
- It accepts any casing for the LifetimeActionType (e.g.: "ROTATE" or "NoTiFy"), delegates the operation to DP, then returns to the caller whatever response came from DP.
- So CP is currently using lowercase because of the regression with DP, which changed to use lowercase.
- When DP is reverted to go back to "Rotate"/"Notify", we should automatically see the same behavior with CP. I did not make any changes to the CP service.
Lets Add CP to it, so we are consistent.
@jlichwa is that actually what CP returns though? In a previous conversation, you indicated that DP was likely changed to align with CP.
I checked CP has lower case. We aligned service to Swagger specs which all had lower case, as we now change service back to PascalCase- I want it to be consistent everywhere, so swagger needs to be updated as well as service change rolled back. I believe @adarce created separate PR for CP
CP's implementation does not attempt to perform any normalization for casing.
- It accepts any casing for the LifetimeActionType (e.g.: "ROTATE" or "NoTiFy"), delegates the operation to DP, then returns to the caller whatever response came from DP.
- So CP is currently using lowercase because of the regression with DP, which changed to use lowercase.
- When DP is reverted to go back to "Rotate"/"Notify", we should automatically see the same behavior with CP. I did not make any changes to the CP service.
Great so just the swagger update on CP - in general it is ok to accept anything, the only thing return value has to be normalized to swagger to align with REST API guidelines for DP.
CP should not be changed. It's a breaking change. They may be inconsistent, but that is how the service behavior shipped and should be retained. Only the swaggers should be updated. CP is still case sensitive - only case-insensitive URLs are allowed. /cc @JeffreyRichter
@heaths it maybe some misunderstanding. We definetely not allow or build different behavior, names for same objects. Today CP is routed through DP and CP is returning for DP provides. If we change DP it automatically flows to CP, if we updating swagger we need both
Talking offline, I now understand that both CP and DP were returning Pascal case, so this is fixing the swagger for both since the service behavior for both is being reverted to what it was.
@weshaggard @maririos do you know who owns the failing azure-powershell? Looking at the results, I'm not entirely sure on the problem but wonder if it's related to the intentional breaking change to the swagger to align with what the service actually returns:
10:19:31.479 cmdout [psci.ps1] keyvault.DefaultTag
10:19:31.479 cmdout [psci.ps1] /mnt/vss/_work/1/s/azure-powershell/swaggerci/keyvault.DefaultTag
10:19:31.479 cmdout [psci.ps1] specification/keyvault/resource-manager/readme.md
10:19:31.479 cmdout [psci.ps1] =================================
10:19:43.737 cmdout [psci.ps1] WARNING: Azure PowerShell CI validation failed for Az.keyvault.DefaultTag
10:19:44.742 cmderr [psci.ps1] InvalidOperation: You cannot call a method on a null-valued expression.
10:19:44.742 cmderr [psci.ps1]
10:19:45.607 cmderr [psci.ps1] InvalidOperation: You cannot call a method on a null-valued expression.
10:19:45.607 cmderr [psci.ps1]
10:19:47.215 cmderr [psci.ps1] InvalidOperation: You cannot call a method on a null-valued expression.
10:19:47.216 cmderr [psci.ps1]
10:19:48.113 cmderr [psci.ps1] InvalidOperation: You cannot call a method on a null-valued expression.
10:19:48.113 cmderr [psci.ps1]
10:19:49.785 cmderr [psci.ps1] MethodInvocationException: Exception calling "Load" with "1" argument(s): "While scanning a plain scalar,
10:19:49.785 cmderr [psci.ps1] found a tab character that violate indentation."
10:19:49.785 cmderr [psci.ps1]
10:19:49.787 cmderr [psci.ps1] InvalidOperation: You cannot call a method on a null-valued expression.
10:19:49.787 cmderr [psci.ps1]
@dingmeng-xue do you know what exactly is causing these errors? I worry it's the case change but, if that was the case - if Az.KeyVault was case-sensitive for these values based solely on the swagger - then this never should've worked. The service team is making the swagger match what the service always returned, which is the REST API Breaking Change Board's recommendation in these cases.
To note, I think this is otherwise good to go but we need to better understand the impact to Az.KeyVault.
/cc @dolauli @isra-fel
@adarce what about https://github.com/Azure/azure-rest-api-specs/blob/fdd4cbaf294814a69131d329d4fd4fb9bccaf6bf/specification/keyvault/resource-manager/Microsoft.KeyVault/stable/2023-02-01/keysManagedHsm.json#L546-L549?
@adarce what about
https://github.com/Azure/azure-rest-api-specs/blob/fdd4cbaf294814a69131d329d4fd4fb9bccaf6bf/specification/keyvault/resource-manager/Microsoft.KeyVault/stable/2023-02-01/keysManagedHsm.json#L546-L549
?
@kim-soohwan : Can you comment on the casing of these enum values returned by MHSM's data plane?
Managed HSM was returning what was in the swagger i.e., camelCase:
{
"attributes": {
"created": 1689274125,
"expiryTime": "P2Y",
"updated": 1689274125
},
"id": "keys/{key-name}/rotationpolicy",
"lifetimeActions": [
{
"action": {
"type": "rotate"
},
"trigger": {
"timeAfterCreate": "P18M"
}
}
]
}
@JeffreyRichter @mikekistler given the Key Vault and Managed HSM APIs are supposed to match and Key Vault recently made a breaking change to match the swagger (camelCase), should we actually keep the breaking service behavior change where Key Vault changed to return camelCase to match the swagger (and Managed HSM). We could make the SDK parse case-insensitive responses to handle either case for this particular enum, which I feel is safer.
The feature has been GA'd since November '22. @jackrichins and Nicholas are looking into telemetry for usage.
/cc @pallavit @jlichwa
I think the service should (as quickly as possible) revert its behavior back to what it was to avoid breaking customers. Treating the enum values as case-insensitive is not a solution:
- Existing customer does not treat it this way
- It is very annoying for customers to do case-insensitive comparisons and they just will not do. We know this because ARM treats URLs as case-insensitive (because it was supposed to be a "convenience") and we get many, many customer complaints about casing issues because it is not a convenience as customers do not know what casing to expect back and their code doesn't handle it correctly.
Hi, @adarce. Your PR has no update for 14 days and it is marked as stale PR. If no further update for over 14 days, the bot will close the PR. If you want to refresh the PR, please remove no-recent-activity label.
![microsoft-github-policy-service[bot] avatar](https://avatars.githubusercontent.com/in/95686?v=4 "Published by a pub.dev verified publisher"){kind=small}
We still have a bunch of Swagger LintDif errors to resolve here around enums shouldn't have duplicate values, when ignoring casing.
Are we doing this on purpose because we propose to continue allowing multiple casings, in the API? Can the SDK codegen handle such things OK? Are we going to add suppressions?
"LifetimeActionsType": {
"properties": {
"type": {
"type": "string",
"description": "The type of the action.",
"enum": [
"Rotate",
"rotate",
"Notify"
],
Please address or respond to feedback from the ARM API reviewer.
When you are ready to continue the ARM API review, please remove the ARMChangesRequested label.
This will notify the reviewer to have another look.
If the feedback provided needs further discussion, please use this Teams channel to post your questions - aka.ms/azsdk/support/specreview-channel.
Please include [ARM Query] in the title of your question to indicate that it is ARM-related.
Hi @adarce! Your PR has some issues. Please fix the CI issues, if present, in following order: Avocado, SemanticValidation, ModelValidation, Breaking Change, LintDiff.
| Task | How to fix | Priority |
|---|---|---|
| Avocado | Fix-Avocado | High |
| Semantic Validation | Fix-SemanticValidation-Error | High |
| Model Validation | Fix-ModelValidation-Error | High |
| LintDiff | Fix-LintDiff | High |
If you need further help, please reach out on the Teams channel aka.ms/azsdk/support/specreview-channel.
Enum values should always be compared in a case sensitive way. Let's not add ROTATE, rotatE, etc.
Azure has gotten into so much trouble for trying to be case-insensitive. Almost all programming languages are case-sensitive, JSON properties are case-sensitive, Linux commands and file system is case-sensitive. Azure should be case-sensitive too.
- Jeffrey Richterhttps://www.linkedin.com/in/jeffrichter/ (see Architecting Distributed Cloud Appshttp://aka.ms/RichterCloudApps & Designing HTTP APIshttps://www.youtube.com/watch?v=9Ng00IlBCtw&list=PL9XzOCngAkqs4m0XdULJu_78nM3Ok3Q65&index=1)
Azure API Stewardship: Guidelineshttps://aka.ms/azapi/guidance | Review process https://aka.ms/azapi | Recordingshttps://aka.ms/azapi/teams | Calendarhttps://aka.ms/azapi/calendar | Join the Interest Grouphttps://aka.ms/azapi/join-interest-group
Azure SDK Architecture: Guidelineshttps://azure.github.io/azure-sdk/general_introduction.html | Review process https://aka.ms/azsdk/archreview/process | Recordingshttps://aka.ms/azsdk/archreview/recordings | Calendarhttps://aka.ms/azsdk/archreview/calendar | Join the Interest Grouphttps://aka.ms/azsdk/archreview/join-interest-group
From: Tim Lovell-Smith @.> Sent: Tuesday, August 15, 2023 1:10 PM To: Azure/azure-rest-api-specs @.> Cc: Jeffrey Richter @.>; Mention @.> Subject: Re: [Azure/azure-rest-api-specs] Align the casing of LifetimeActionsType with the service. (PR #24475)
We still have a bunch of Swagger LintDif errors to resolve here around enums shouldn't have duplicate values, when ignoring casing.
Are we doing this on purpose because we propose to continue allowing multiple casings, in the API? Can the SDK codegen handle such things OK? Are we going to add suppressions?
"LifetimeActionsType": { "properties": { "type": { "type": "string", "description": "The type of the action.", "enum": [ "Rotate", "rotate", "Notify" ],
— Reply to this email directly, view it on GitHubhttps://github.com/Azure/azure-rest-api-specs/pull/24475#issuecomment-1679539245, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AARLJP3SMYCIWU45CCFQDBTXVPJT5ANCNFSM6AAAAAAZIRPUBA. You are receiving this because you were mentioned.Message ID: @.***>
@jeffreyrichter I'm also against adding more case variations in the enum definition - its very confusing (and exponential complexity) - but I'm trying to clarify, if you are meaning that we should hereafter declare exactly two different supported casings of 'rotate', or just one?
Only ever just 1 casing. Usually Pascal casing, so "Rotate".
- Jeffrey Richterhttps://www.linkedin.com/in/jeffrichter/ (see Architecting Distributed Cloud Appshttp://aka.ms/RichterCloudApps & Designing HTTP APIshttps://www.youtube.com/watch?v=9Ng00IlBCtw&list=PL9XzOCngAkqs4m0XdULJu_78nM3Ok3Q65&index=1)
Azure API Stewardship: Guidelineshttps://aka.ms/azapi/guidance | Review process https://aka.ms/azapi | Recordingshttps://aka.ms/azapi/teams | Calendarhttps://aka.ms/azapi/calendar | Join the Interest Grouphttps://aka.ms/azapi/join-interest-group
Azure SDK Architecture: Guidelineshttps://azure.github.io/azure-sdk/general_introduction.html | Review process https://aka.ms/azsdk/archreview/process | Recordingshttps://aka.ms/azsdk/archreview/recordings | Calendarhttps://aka.ms/azsdk/archreview/calendar | Join the Interest Grouphttps://aka.ms/azsdk/archreview/join-interest-group
From: Tim Lovell-Smith @.> Sent: Tuesday, August 15, 2023 2:11 PM To: Azure/azure-rest-api-specs @.> Cc: Jeffrey Richter @.>; Mention @.> Subject: Re: [Azure/azure-rest-api-specs] Align the casing of LifetimeActionsType with the service. (PR #24475)
@JeffreyRichterhttps://github.com/JeffreyRichter I'm also against adding more case variations in the enum definition - its very confusing (and exponential complexity) - but I'm trying to clarify, if you are meaning that we should hereafter declare exactly two different supported casings of 'rotate', or just one?
— Reply to this email directly, view it on GitHubhttps://github.com/Azure/azure-rest-api-specs/pull/24475#issuecomment-1679620402, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AARLJPYCRVMGXYBKW5Y2HJ3XVPQW5ANCNFSM6AAAAAAZIRPUBA. You are receiving this because you were mentioned.Message ID: @.***>
We still have a bunch of Swagger LintDif errors to resolve here around enums shouldn't have duplicate values, when ignoring casing.
Are we doing this on purpose because we propose to continue allowing multiple casings, in the API? Can the SDK codegen handle such things OK? Are we going to add suppressions?
"LifetimeActionsType": { "properties": { "type": { "type": "string", "description": "The type of the action.", "enum": [ "Rotate", "rotate", "Notify" ],
This is intentional, yes. Managed HSM and Key Vault return different casing and this is what was decided by the REST API Stewardship Board was "less bad". @adarce, when can we epxect changes? Basically, all the enums should be something like:
"LifetimeActionsType": {
"properties": {
"type": {
"type": "string",
"description": "The type of the action.",
"enum": [
"Rotate",
"rotate",
"Notify"
],
"x-ms-enum": {
"name": "LifetimeActionsType",
"modelAsString": true,
"values": [
{
"value": "Rotate",
"description": "Rotate the key based on the key policy. Key Vault only. Managed HSM uses camelCase 'rotate' instead."
},
{
"value": "rotate",
"description": "Rotate the key based on the key policy. Managed HSM only. Key Vault uses PascalCase 'Rotate' instead."
},
{
"value": "Notify",
"description": "Trigger Event Grid events. Defaults to 30 days before expiry. Key Vault only."
}
]
}
}
}
}
The SDKs - except for Go - will normalize the values, though. There will be no public API changes to the SDKs.