Skip to content

Conversation

@nated0g
Copy link
Contributor

@nated0g nated0g commented Sep 5, 2025

Issue #, if available:

Description of changes:

The initializer of the HTTPAPIKeyAuthTrait as currently implemented is incorrect. As it is currently written, initialization will always fail. The httpApiKeyAuth trait in the generated schema code ends up looking like this:

Trait.new(
    id=ShapeID("smithy.api#httpApiKeyAuth"),
    value=MappingProxyType({"name": "Authorization", "in": "header"}),
),

When building:

  File "/app/smithy/build/conversations/python-client-codegen/src/conversations/_private/schemas.py", line 1036, in <module>
    Trait.new(
    ~~~~~~~~~^
        id=ShapeID("smithy.api#httpApiKeyAuth"),
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        value=MappingProxyType({"name": "Authorization", "in": "header"}),
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    ),
    ^
  File "/app/.venv/lib/python3.13/site-packages/smithy_core/traits.py", line 91, in new
    return cls(value)
  File "/app/.venv/lib/python3.13/site-packages/smithy_core/traits.py", line 338, in __init__
    object.__setattr__(self, "location", APIKeyLocation(value))
                                         ~~~~~~~~~~~~~~^^^^^^^
  File "/usr/local/lib/python3.13/enum.py", line 726, in __call__
    return cls.__new__(cls, value)
           ~~~~~~~~~~~^^^^^^^^^^^^
  File "/usr/local/lib/python3.13/enum.py", line 1203, in __new__
    raise ve_exc
ValueError: mappingproxy({'name': 'Authorization', 'in': 'header'}) is not a valid APIKeyLocation

The current implementation tries to construct an APIKeyLocation from this entire object, instead of from the required in property (https://smithy.io/2.0/spec/authentication-traits.html#httpapikeyauth-trait), as it should.

This change constructs the APIKeyLocation correctly from the in property


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@nated0g nated0g requested a review from a team as a code owner September 5, 2025 17:52
Copy link
Contributor

@jonathan343 jonathan343 left a comment

Choose a reason for hiding this comment

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

Hey @nated0g,

Thanks for opening this pull request!
I think this looks good but the type checker is probably gonna complain since this is being added before the assert isinstance(self.document_value, Mapping) line.

I'll run our CI so you can see what I'm taking about.
All you'll need to do is move these lines below the one I mentioned above.

@nated0g nated0g force-pushed the fix/http-api-key-auth-trait branch from 8dc4832 to 87eeb68 Compare September 5, 2025 20:05
@nated0g
Copy link
Contributor Author

nated0g commented Sep 5, 2025

Hey @nated0g,

Thanks for opening this pull request! I think this looks good but the type checker is probably gonna complain since this is being added before the assert isinstance(self.document_value, Mapping) line.

I'll run our CI so you can see what I'm taking about. All you'll need to do is move these lines below the one I mentioned above.

Good call, done.

@nated0g nated0g requested a review from jonathan343 September 5, 2025 20:24
Copy link
Contributor

@jonathan343 jonathan343 left a comment

Choose a reason for hiding this comment

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

LGTM!

@jonathan343 jonathan343 merged commit b218ed2 into smithy-lang:develop Sep 5, 2025
4 checks passed
@jonathan343
Copy link
Contributor

Thanks again @nated0g!
I'll add a bugfix changelog entry for this in #552

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants