Skip to content

Conversation

lauzadis
Copy link
Contributor

Issue #

Description of changes

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

lauzadis and others added 30 commits November 14, 2024 13:03
Co-authored-by: 0marperez <60363173+0marperez@users.noreply.github.com>
Co-authored-by: aws-sdk-kotlin-ci <aws-kotlin-sdk-automation@amazon.com>
Co-authored-by: Ian Botsford <83236726+ianbotsf@users.noreply.github.com>
Co-authored-by: Xinsong Cui <xicu@amazon.com>
Co-authored-by: Manuel Sugawara <sugmanue@amazon.com>
Comment on lines 119 to 120
override fun inputStream(): InputStream = inner.inputStream()
override fun outputStream(): OutputStream = inner.outputStream()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are now exposed as extension functions, see runtime/runtime-core/jvm/src/aws/smithy/kotlin/runtime/io/JavaIO.kt

Comment on lines 907 to 916
public abstract interface class aws/smithy/kotlin/runtime/io/SdkBufferedSink : aws/smithy/kotlin/runtime/io/SdkSink, java/nio/channels/WritableByteChannel {
public abstract interface class aws/smithy/kotlin/runtime/io/SdkBufferedSink : aws/smithy/kotlin/runtime/io/SdkSink {
public abstract fun emit ()V
public abstract fun flush ()V
public abstract fun getBuffer ()Laws/smithy/kotlin/runtime/io/SdkBuffer;
public abstract fun outputStream ()Ljava/io/OutputStream;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We renamed SdkBufferedSinkJvm to SdkBufferedSinkJvmAndNative which no longer implements WritableByteChannel. This removes the outputStream() API from the interface, but we now expose it in JavaIO as an extension function. Same package, so it's not a break?

Copy link
Contributor

Choose a reason for hiding this comment

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

Definitely a break. The fact that this interface no longer inherits from WritableByteChannel means callers can no longer pass it into fields/args of that type. Additionally, extension functions must be imported to be used and this changes the import requirement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed by splitting this back into Jvm and Native sourcesets like we discussed. We can explore combining them again in a future minor version bump

Comment on lines 932 to 940
public abstract interface class aws/smithy/kotlin/runtime/io/SdkBufferedSource : aws/smithy/kotlin/runtime/io/SdkSource, java/nio/channels/ReadableByteChannel {
public abstract interface class aws/smithy/kotlin/runtime/io/SdkBufferedSource : aws/smithy/kotlin/runtime/io/SdkSource {
public abstract fun exhausted ()Z
public abstract fun getBuffer ()Laws/smithy/kotlin/runtime/io/SdkBuffer;
public abstract fun inputStream ()Ljava/io/InputStream;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We renamed SdkBufferedSourceJvm to SdkBufferedSourceJvmAndNative which no longer implements ReadableByteChannel. This removes the inputStream() API from the interface, but we now expose it in JavaIO.kt as an extension function. Same package, so it's not a break?

@lauzadis lauzadis added the acknowledge-api-break Acknowledge that a change is API breaking and may be backwards-incompatible. Review carefully! label Sep 29, 2025
@lauzadis
Copy link
Contributor Author

Added acknowledge-api-break, but I've addressed all the real API breaks as we discussed. Only false positives remain

@lauzadis lauzadis marked this pull request as ready for review September 29, 2025 14:49
@lauzadis lauzadis requested a review from a team as a code owner September 29, 2025 14:49
@github-actions

This comment has been minimized.

…ient/native/src/aws/smithy/kotlin/runtime/client/util/AnnotationsNative.kt:8:8 Annotation `@Retention(value = AnnotationRetention.SOURCE)` is missing on actual declaration.

         All annotations from expect `aws.smithy.kotlin.runtime.client.util.MpJvmDefaultWithoutCompatibility` must be present with the same arguments on actual `aws.smithy.kotlin.runtime.client.util.MpJvmDefaultWithoutCompatibility`, otherwise they might behave incorrectly.
@github-actions
Copy link

Affected Artifacts

Changed in size
Artifact Pull Request (bytes) Latest Release (bytes) Delta (bytes) Delta (percentage)
http-test-jvm.jar 62,181 59,427 2,754 4.63%
crt-util-jvm.jar 21,733 21,055 678 3.22%
aws-signing-default-jvm.jar 68,505 66,941 1,564 2.34%
runtime-core-jvm.jar 852,677 835,526 17,151 2.05%
http-client-engine-crt-jvm.jar 54,233 53,980 253 0.47%
aws-signing-crt-jvm.jar 16,114 16,081 33 0.21%
test-suite-jvm.jar 99,844 100,085 -241 -0.24%

@lauzadis lauzadis merged commit abf545f into main Sep 29, 2025
38 checks passed
@lauzadis lauzadis deleted the kn-main branch September 29, 2025 19:56
lauzadis added a commit that referenced this pull request Sep 30, 2025
lauzadis added a commit that referenced this pull request Sep 30, 2025
lauzadis added a commit that referenced this pull request Oct 1, 2025
lauzadis added a commit that referenced this pull request Oct 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

acknowledge-api-break Acknowledge that a change is API breaking and may be backwards-incompatible. Review carefully!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants