Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modify Enum conversions to be case-insensitive/case-correcting #3880

Merged
merged 23 commits into from
Mar 24, 2024

Conversation

theunrepentantgeek
Copy link
Member

What this PR does / why we need it:

In some cases information returned about a resource is is inconsistent with the letter casing specified in the OpenAPI specs for a resource - for example, the specs might specify Succeeded but the service returns succeeded.

This PR modifies enum conversion to work in a case-insensitive way, which will case-correct values to conform with the OpenAPI specs.

Special notes for your reviewer:

The conversion from status to spec used by asoctl will also pick up this fix.

Closes #3805

How does this PR make you feel:
gif

@codecov-commenter
Copy link

codecov-commenter commented Mar 18, 2024

Codecov Report

Attention: Patch coverage is 99.85577% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 53.70%. Comparing base (f66dd4e) to head (aa111dd).
Report is 2 commits behind head on main.

Files Patch % Lines
...2/api/apimanagement/v1api20220801/api_types_gen.go 89.28% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3880      +/-   ##
==========================================
+ Coverage   53.53%   53.70%   +0.16%     
==========================================
  Files        1523     1525       +2     
  Lines      550082   552819    +2737     
==========================================
+ Hits       294510   296878    +2368     
- Misses     210035   210392     +357     
- Partials    45537    45549      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@theunrepentantgeek theunrepentantgeek force-pushed the feature/case-insensitive-enums branch from 22c04f4 to c00fb31 Compare March 21, 2024 03:22
@theunrepentantgeek theunrepentantgeek force-pushed the feature/case-insensitive-enums branch from c00fb31 to b233fab Compare March 21, 2024 03:50
@theunrepentantgeek theunrepentantgeek force-pushed the feature/case-insensitive-enums branch from d3f7467 to aa111dd Compare March 22, 2024 00:53

// ToEnum does a case-insensitive conversion of a string to an enum using a provided conversion map.
// If the required value is not found, a literal cast will be used to return the enum.
func ToEnum[T ~string](str string, enumMap map[string]T) T {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. Full credit though - you gave me the method signature. 😁

@@ -298,6 +298,11 @@ type FakeResource_APIVersion_Spec string

const FakeResource_APIVersion_Spec_20200601 = FakeResource_APIVersion_Spec("2020-06-01")

// Mapping from string to FakeResource_APIVersion_Spec
var fakeResource_APIVersion_Spec_Values = map[string]FakeResource_APIVersion_Spec{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any idea why this apiversion in the test isn't getting excluded?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the type isn't called exactly APIVersion - I'm guessing because the name isn't being simplified in the same way as the main pipeline.

@theunrepentantgeek theunrepentantgeek added this pull request to the merge queue Mar 24, 2024
Merged via the queue into main with commit 8a7995a Mar 24, 2024
8 checks passed
@theunrepentantgeek theunrepentantgeek deleted the feature/case-insensitive-enums branch March 24, 2024 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Bug: asoctl import-ing a ManagedCluster produces an invalid resource
3 participants