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

fix: apply clock skew interceptor to clients created via invoke #1067

Merged
merged 2 commits into from
Apr 17, 2024

Conversation

ianbotsf
Copy link
Contributor

@ianbotsf ianbotsf commented Apr 16, 2024

Issue #

Addresses awslabs/aws-sdk-kotlin#1211

Description of changes

This change supports adding the clock skew interceptor to clients created via invoke (as opposed to via fromEnvironment) by creating a new AbstractSdkClientFactory superclass with a non-suspend finalizeConfig method and adjusting codegen accordingly.

Companion PR: awslabs/aws-sdk-kotlin#1284

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Comment on lines +19 to +22
public override operator fun invoke(block: TConfigBuilder.() -> Unit): TClient = builder().apply {
config.apply(block)
finalizeConfig(this)
}.build()
Copy link
Contributor

Choose a reason for hiding this comment

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

Invoking an abstract class looks a little odd since they aren't meant to be instantiated. I don't have any suggestion here, it just stands out to me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This isn't quite invoking an abstract class. It's an operator defined on an abstract class which will only be callable on a concrete instance. No one's ever going to call AbstractSdkClientFactory.invoke { … } or AbstractSdkClientFactory().invoke { … }—both are compile-time errors.

This simply enables classes which inherit AbstractSdkClientFactory (namely, service client companion objects) to be invoked (e.g., S3Client.invoke { … }).

@ianbotsf ianbotsf changed the base branch from main to v1.2 April 17, 2024 21:09
@ianbotsf
Copy link
Contributor Author

Discussed offline and this change is breaking enough that we don't want to push it directly to main. Updated base branch to v1.2. Will pursue a temporary fix strictly within aws-sdk-kotlin codegen until such time as we have another minor version bump.

@ianbotsf ianbotsf merged commit b0ef3c2 into v1.2 Apr 17, 2024
13 checks passed
@ianbotsf ianbotsf deleted the fix-clockskew-application branch April 17, 2024 21:11
lauzadis added a commit that referenced this pull request Apr 25, 2024
* fix: apply clock skew interceptor to clients created via `invoke` (#1067)

* bumping version to 1.2.0

* whoops wait, don't bump versions manually; use the changelog mechanism 'requiresMinorVersionBump'

* misc: upgrade to OkHttp 5.0.0-alpha.14, Okio 3.9.0, Kotlin 1.9.23 (#1070)

* fix: remove draft (#1073)

---------

Co-authored-by: Ian Botsford <83236726+ianbotsf@users.noreply.github.com>
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