Skip to content

Conversation

@alextwoods
Copy link
Contributor

Description of changes:
Based on suggestion in #394, this PR implements StandardRegionalEndpoints using CodeSections + interceptors.

Similar to the HttpAuthGenerator, we add a standard EndpointGenerator which always renders endpoints.py which must define the service's EndpointParameters and EndpointResolver[EndpointParameters] and which adds code sections for both.

The AWSStandardRegionalIntegation adds the required config properties through a runtime plugin and adds interceptors that completely rewrite the endpoint param and resolver sections. For both the base and standard regional cases, we simply import generic (non service-specific) parameters and resolvers and export them. For endpoints2.0 we would generate both in place.

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I hate that spotless reformatted all of this. It was fairly deliberately formatted to keep everything at a level so it could be read with minimal head scratching. But we're getting rid of these soon anyway so oh well.

Copy link
Contributor Author

@alextwoods alextwoods Mar 10, 2025

Choose a reason for hiding this comment

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

Ugh, Yeah, sorry about that :-(

For this PR I've gone ahead and reverted all of those formatting changes to make sure the actual change is clear.

Comment on lines 15 to 21
Config = TypeVar("Config", contravariant=True)


class EndpointParametersProtocol(Protocol[Config]):
@classmethod
def build(cls, config: Config) -> Self:
raise NotImplementedError()
Copy link
Contributor

Choose a reason for hiding this comment

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

As of python 3.12 we have proper generic declarations and inferred variance, so TypeVar is not needed.

Suggested change
Config = TypeVar("Config", contravariant=True)
class EndpointParametersProtocol(Protocol[Config]):
@classmethod
def build(cls, config: Config) -> Self:
raise NotImplementedError()
class EndpointParameters[C](Protocol):
@classmethod
def build(cls, config: C) -> Self:
raise NotImplementedError()



class EndpointResolver(Protocol[EndpointParams]):
class EndpointResolverProtocol(Protocol[EndpointParams]):
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding type information in the name of the type isn't a thing we should be doing. Especially in Python, where you can just alias the name on import to avoid conflicts.

@alextwoods alextwoods marked this pull request as ready for review March 10, 2025 16:51
@alextwoods alextwoods requested a review from a team as a code owner March 10, 2025 16:51
@alextwoods alextwoods merged commit 2d86175 into smithy-lang:develop Mar 11, 2025
2 checks passed
@alextwoods alextwoods deleted the regional_endpoints_2 branch March 11, 2025 15:20
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