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

Remove dependency from aws sdk v1 #1163

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

abhit17
Copy link
Contributor

@abhit17 abhit17 commented Jul 7, 2023

Description of changes:

Removing dependency from aws sdk v1 for multilang deamon. KCL is already updated.

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

Testing

  1. Tested by running mvn verify
  2. Tested by running python kcl on this updated multi lang deamon.

Logs for mvn verify

[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for Amazon Kinesis Client Library 2.5.1:
[INFO]
[INFO] Amazon Kinesis Client Library ...................... SUCCESS [  2.145 s]
[INFO] Amazon Kinesis Client Library for Java ............. SUCCESS [15:07 min]
[INFO] amazon-kinesis-client-multilang .................... SUCCESS [ 12.868 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  15:23 min
[INFO] Finished at: 2023-07-06T19:28:11-07:00
[INFO] ------------------------------------------------------------------------

@abhit17 abhit17 assigned abhit17 and chenylee-aws and unassigned abhit17 Jul 7, 2023
Copy link
Contributor

@chenylee-aws chenylee-aws left a comment

Choose a reason for hiding this comment

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

I see there are still some reference to the V1 package (com.amazonaws) for example here. We might want to clean them up as well but I don't know if this will break support for people who are still using SDK V1 in other language through multilang.

@stair-aws
Copy link
Contributor

FYI, related issue: #1161

@stair-aws stair-aws added the v2.x Issues related to the 2.x version label Jul 7, 2023
@junyuc25
Copy link

Hi @stair-aws , it looks like the one of the modules is transitively depending on SDK v1, as shown below:
mvn dependency:tree -Dincludes="com.amazonaws:*"

[INFO] software.amazon.kinesis:amazon-kinesis-client:jar:2.5.1
[INFO] \- software.amazon.glue:schema-registry-serde:jar:1.1.14:compile
[INFO]    \- com.amazonaws:aws-java-sdk-sts:jar:1.12.151:compile
[INFO]       +- com.amazonaws:aws-java-sdk-core:jar:1.12.151:compile
[INFO]       \- com.amazonaws:jmespath-java:jar:1.12.151:compile

Please correct me if I'm wrong. It seems to me that, without first upgrading schema-registry-serde, KCL could not be fully migrated to SDK v2.

@stair-aws
Copy link
Contributor

@junyuc25 Thanks for that callout! Glue is still anchored to SDK v1:

        <dependency>
            <groupId>com.amazonaws</groupId>
            <artifactId>aws-java-sdk-sts</artifactId>
            <version>${aws.sdk.v1.version}</version>
        </dependency>

I will update our internal tracker with information to contact the Glue maintainers.

@stair-aws
Copy link
Contributor

[junyuc25@] It seems to me that, without first upgrading schema-registry-serde, KCL could not be fully migrated to SDK v2.

Mostly agree, yet disagree on "first". We should be able to revise KCL independent from Glue -- and revbump serde later -- yet both will need updates to holistically remove SDK v1. This transitive inclusion of SDK v1 explains chenylee-aws@'s observation.

@stair-aws
Copy link
Contributor

While we can modify the KCL code to not depend on SDKv1, we likely cannot remove the dependency on SDKv1 -- direct or transitive -- outside of a major release. Multilang customers may exist that utilize AWSCredentialsProviders via properties file AWSCredentialsProvider = <SDKv1 class name> inclusion (see sample.properties).

AWSCredentialsProviderPropertyValueDecoder maps a simple class name (e.g., STSAssumeRoleSessionCredentialsProvider) to its fully-qualified SDKv1 class.

@junyuc25
Copy link

Hi @stair-aws , can we prepare for a major release for the SDK upgrade? And how much efforts will it take to release a major version?

@stair-aws
Copy link
Contributor

can we prepare for a major release for the SDK upgrade? And how much efforts will it take to release a major version?

@junyuc25 Salut! Let's bring this conversation internally so we can discuss in depth. I responded to your DM and commented on our team ticket. Thanks!

@mrdziuban
Copy link
Contributor

Hey @stair-aws and @junyuc25, are there any updates you could share on this? Thanks in advance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v2.x Issues related to the 2.x version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants