aws / aws-sdk

Landing page for the AWS SDKs on GitHub
https://aws.amazon.com/tools/
Other
68 stars 13 forks source link

AppConfig EnvironmentState enum constants incorrect definitions #592

Closed DerkSchooltink closed 3 months ago

DerkSchooltink commented 10 months ago

Describe the bug

It seems like the enum constants of EnvironmentState of the AppConfig service follow a different pattern:

$ aws appconfig list-environments --application-id=...
{
    "Items": [
        {
            "ApplicationId": "...",
            "Id": "...",
            "Name": "...",
            "State": "RolledBack",
            "Monitors": []
        },
        {
            "ApplicationId": "...",
            "Id": "...",
            "Name": "...",
            "State": "ReadyForDeployment",
            "Monitors": []
        }
    ]
}

All other enum constants seem to be using an UPPER_SNAKE_CASE naming strategy, only EnvironmentState is using a regular CamelCase naming strategy.

Considering the inconsistency with other enums in the same domain, I figured this is more than just bad mapping by Smithy definitions. It could be a bug in AppConfig itself. Seeing that the contributor guidelines mention that you do not accept PR's for the Smithy definitions, I figured I'd log this bug report instead. If there's a need to escalate the issue somewhere else (for a change in the AppConfig ListEnvironments/GetEnvironment API's) then let me know :)

The issue is more widespread and not just contained to the SDK. Also public documentation lists the constants as UPPER_SNAKE_CASE:

  1. CLI documentation
  2. Get Environment API documentation

Reproduction Steps

In my local project I'm using the following mapper:


func mapEnvironmentState(state types.EnvironmentState) model.EnvironmentState {
    switch state {
    case types.EnvironmentStateDeploying:
        return model.StateDeploying
    case types.EnvironmentStateReadyForDeployment:
        return model.StateReadyForDeployment
    case types.EnvironmentStateRolledBack:
        return model.StateRolledBack
    case types.EnvironmentStateRollingBack:
        return model.StateRollingBack
    default:
        logrus.Warnf("could not map state %s, using default empty string", state)
        return ""
    }
}

Which will always fall back to the default case, because the constants provided by the SDK are in UPPER_SNAKE_CASE instead of CamelCase.

Compiler and Version used

go version go1.21.0 darwin/arm64

Operating System and version

MacOs 13.4.1

RanVaknin commented 10 months ago

Hi @DerkSchooltink ,

Thanks for raising this issue. I'm able to confirm that this is a problem. The problem stems from the fact that the AppConfig service team incorrectly modeled the enums as snake case (as you have pointed out).

That is evident from the internal model the SDK uses to generate the code. Unfortunately the AppConfig service team is the only entity that has direct control over the model, and the SDK team cannot do anything to fix this.

I have created an internal ticket with the Appconfig service team to mitigate this issue.

Stay tuned. Thanks, Ran~

P97886363

DerkSchooltink commented 10 months ago

@RanVaknin thank you for the elaboration, I figured this was not an issue we could solve SDK-side, but I wasn't sure where else to report it. :)

Is there any merit in keeping this issue open? What will be the way forward?

bhoradc commented 3 months ago

Hello @DerkSchooltink,

The service team has acknowledged this issue and they have a backlog item created for tracking it.

There is no ETA on the fix for now. Therefore, I will go ahead and close this issue.

Regards, Chaitanya

github-actions[bot] commented 3 months ago

This issue is now closed.

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.