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

Adding Async support for S3, so you can use it with spring-cloud-aws. #50

Closed
wants to merge 2 commits into from

Conversation

varunmehta
Copy link

*Issue: #5

Description of changes:
Adding Async support for S3, so you can integrate it with spring-cloud-aws @aws-aminsuzani

The change has to be done at the core library level for the teams to use it; spring-attic/spring-cloud-aws#167

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

forked their version, and added my code on top of that.

TODO:
1 .add tests to verify the new code.
2. Identify which other methods from AmazonSQSAsyncClient are used,
which could have S3 attributes, which need to be extended and translated
for spring-cloud-aws.
@dexwest
Copy link

dexwest commented Aug 3, 2020

I need this is there any chance this is close to be merged?

@ajross
Copy link

ajross commented Aug 11, 2020

I suggest this isn't quite ready to be merged. I've been trying to use it and some additional functionality should be added to support encrypted S3 buckets (though arguably that's an issue with the AWS ExtendedClientConfiguration class). Additionally, in the current implementation, when calling the constructor with your own ExtendedClientConfiguration, the config isn't passed through to the client that's used for sending the message. A change needs to be made to the constructors in the AmazonSQSAsyncExtendedClient to call super(sqsClient, extendedClientConfig). If I get it working I'll see if I can contribute the change back.

@sreekanth-tf
Copy link

@ajross may I know, is there any update?

@richardsmithsfdc
Copy link
Contributor

I see this was attempted a few times, and I wanted to add my attempt into the ring here. This is a draft PR to add async support to extended client library: #79 , which is based on async support in payload offloading, for which I also created a PR: awslabs/payload-offloading-java-common-lib-for-aws#19 .

This is a full async implementation, with design consistent with the sync implementation. It is accompanied by a full set of UTs, and I have locally done IT to verify that it works.

If anyone is interested in this, please review the payload offloading PR first. Once that is committed, the new PR can be changed to ready state and also reviewed.

I'll add this same comment to the original issue.

@ziyanli-amazon
Copy link
Contributor

Thanks to @richardsmithsfdc the feature will be supported since 2.1.0.

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.

6 participants