Amazon service extensions #4968

marcinczeczko · 2019-10-29T08:50:46Z

I'd like to introduce a broader integration with AWS SDK v2. It consists of:

Amazon services extensions root module
Introduction of Amazon services common module to easily add support for other AWS clients.
Refactored DynamoDB extension to be part of amazon services
Added S3 client extension
Quickstart guide that shows how to store and retrieve file using S3

More clients (SNS, SQS, etc.) to be added if the concept in this PR will be finalised and merged

gsmet · 2019-10-29T10:04:58Z

I cancelled the build, we are scarce on CI resources right now.

gsmet · 2019-11-01T18:45:23Z

Oh man, this is a large one :). It looks very interesting, I need to take more time to think about it and review it.

marcinczeczko · 2019-11-01T18:54:48Z

Yes I know it's quite large :) No worries, we are not in the rush.

marcinczeczko · 2019-11-28T10:50:49Z

@gsmet - Any thoughts on this PR ?

gsmet · 2019-11-28T11:12:20Z

I reviewed the Flyway PR yesterday, this one is next in line once I'm done with releasing 1.0.1.Final so you should have some feedback soon.

Sorry, the 1.0 work got me very late on reviewing large PRs.

gsmet · 2019-12-04T15:31:16Z

@marcinczeczko sorry about the delay. I'm starting to have a look just now.

gsmet

So, in general, that's a big +1. I really like what you have here. Thanks for your work (and for your patience...).

It needs a rebase and some changes to the extension descriptor. The rest is mostly some small comments.

Also I'm not entirely sure about how your architected the configuration. I think we need to discuss it and it would probably be better to do it live if you can.

gsmet · 2019-12-04T15:32:01Z

bom/runtime/pom.xml

                <version>${project.version}</version>
            </dependency>
+
+


Micro comment: I don't think we need these artificial new lines.

gsmet · 2019-12-04T15:34:10Z

ci-templates/stages.yml

            - amazon-lambda-http
          name: amazon
-          dynamodb: true
+          amazonStack: true


Maybe let's call it amazonServices (or even better amazon-services but I'm not sure dashes are allowed).

Only underscores are supported but doesn't looks good, so I kept camelCase with amazonServices as value

gsmet · 2019-12-04T15:34:58Z

docs/src/main/asciidoc/amazon-s3-guide.adoc

+* Blocking access using URL Connection HTTP client (by default) or the Apache HTTP Client
+* https://docs.aws.amazon.com/sdk-for-java/v2/developer-guide/basics-async.html[Asynchronous programming] based on JDK's `CompletableFuture` objects and the Netty HTTP client.
+
+In this guide, we see how you can get your REST services to use the DynamoDB locally and on AWS.


Shouldn't this whole introduction be about S3? :)

I forgot to mention, that S3 guide I'm about to work on later :) it's just a stub atm. Will polish it later this week.

docs/src/main/asciidoc/amazon-s3-guide.adoc

gsmet · 2019-12-04T17:55:36Z

extensions/amazon-services/dynamodb/deployment/pom.xml

+        </dependency>
+        <dependency>
+            <groupId>io.quarkus</groupId>
+            <artifactId>quarkus-amazon-common</artifactId>


This should come with the above, no?

gsmet · 2019-12-04T17:57:10Z

extensions/amazon-services/dynamodb/deployment/src/test/resources/async-full-config.properties

+quarkus.amazon.aws.dynamodb.region=us-east-1
+quarkus.amazon.aws.dynamodb.credentials.type=static
+quarkus.amazon.aws.dynamodb.credentials.static-provider.access-key-id=test-key
+quarkus.amazon.aws.dynamodb.credentials.static-provider.secret-access-key=test-secret


I think we need to have a discussion about this config stuff. Would you have a moment for a call so that we can discuss it live?

gsmet · 2019-12-04T17:58:01Z

extensions/amazon-services/dynamodb/runtime/src/main/resources/META-INF/quarkus-extension.json

+    "dynamo",
+    "aws",
+    "amazon"
+  ]


Same here about the format being YAML now.

gsmet · 2019-12-04T17:59:18Z

extensions/amazon-services/s3/runtime/src/main/resources/META-INF/quarkus-extension.json

+        "s3",
+        "aws",
+        "amazon"
+    ]


Same here about the format.

integration-tests/amazon-services/pom.xml

gsmet · 2020-01-06T12:19:35Z

@marcinczeczko happy new year! Any chance we could restart the progress on this? It looked very promising!

marcinczeczko · 2020-01-07T08:07:49Z

@gsmet Happy new year too !! I just came back from long Christmas break :) I'm gonna to restart that work this week. I think we can talk about in live as you suggested - let's catchup on zulip to get up to speed.

oztimpower · 2020-02-24T14:48:57Z

Any progress on this one?

It's super fiddly and time consuming trying to get basic AWS Java SDK v2 (or v1) integrations working. I have SNS working using the sync client UrlConnectionHttpClient yet couldn't get it working with Apache (SSL issues).

Required reflect-config.json for the UrlConnectionHttpClient:
[
{
"name":"com.sun.xml.internal.stream.XMLInputFactoryImpl",
"methods":[{"name":"","parameterTypes":[] }]
}
]

sherl0cks · 2020-04-08T20:47:27Z

Is there any plans to move this forward @marcinczeczko ?

@gsmet @oscerd @oztimpower I must admit that I worry about the strategy being employed here, which seems to be taken from the initial dynamodb extension. Specifically, all the work being done to actually initiate clients and proxy the AWS SDK configuration model. It is seems unnecessarily complex and hard to maintain, and perhaps why this PR is stalled after months of works and 120 files edited.

As a user deploy service to production with S3, Dynamo, SNS and SQS with quarkus / graal native / aws lambda, all I want quarkus to do is to handle the ugly internals of the build system (e.g. proxy generation, reflection config, resources config, maven dependency management) such that when I create AWS SDK clients in my CDI configuration (which is easy to do), they just work. On occasion it might be necessary to do bytecode replacement to ensure effective integration with quarkus, as well. I don't need the beans created for me. You can see this alternative model in https://github.com/quarkusio/quarkus/tree/master/extensions/amazon-lambda-xray we worked on with @patriot1burke .

I have the above approach working in the applications I'm deploying now to production, and if there was interest in trying this new strategy, then we can discuss how best to build out these features together (along with a guide). I will probably need support from the maintainers give my existing job responsibilities. That said, if maintainers are set on the existing strategy, then I don't want to waste time implementing an alternative.

oscerd · 2020-04-10T11:01:52Z

Is there any plans to move this forward @marcinczeczko ?

@gsmet @oscerd @oztimpower I must admit that I worry about the strategy being employed here, which seems to be taken from the initial dynamodb extension. Specifically, all the work being done to actually initiate clients and proxy the AWS SDK configuration model. It is seems unnecessarily complex and hard to maintain, and perhaps why this PR is stalled after months of works and 120 files edited.

As a user deploy service to production with S3, Dynamo, SNS and SQS with quarkus / graal native / aws lambda, all I want quarkus to do is to handle the ugly internals of the build system (e.g. proxy generation, reflection config, resources config, maven dependency management) such that when I create AWS SDK clients in my CDI configuration (which is easy to do), they just work. On occasion it might be necessary to do bytecode replacement to ensure effective integration with quarkus, as well. I don't need the beans created for me. You can see this alternative model in https://github.com/quarkusio/quarkus/tree/master/extensions/amazon-lambda-xray we worked on with @patriot1burke .

I have the above approach working in the applications I'm deploying now to production, and if there was interest in trying this new strategy, then we can discuss how best to build out these features together (along with a guide). I will probably need support from the maintainers give my existing job responsibilities. That said, if maintainers are set on the existing strategy, then I don't want to waste time implementing an alternative.

Hello, personally I'm working on supporting the AWS SDK v2 in the https://github.com/apache/camel-quarkus and I'm still trying to find a common way between the multiple components. I will post my findings here whenever I have some.

sherl0cks · 2020-04-10T16:07:57Z

@oscerd Interesting. I have some thoughts on this from my work and also a lot of experience with camel (I use to work with Red Hat consulting). I'll send you an email - perhaps we can discuss this over a video call quickly to coordinate?

oscerd · 2020-04-10T17:51:15Z

Sure thing!

marcinczeczko · 2020-04-10T18:13:48Z

Hi @oscard, yes I planned to resume that work. I had to stop for a while because of client work. I think we can discuss it in a wider group.

sherl0cks · 2020-04-10T19:32:56Z

@marcinczeczko sent email to your gmail to coordinate a discussion. Looking forward to discussing with you soon.

marcinczeczko · 2020-05-03T20:00:58Z

@machi1990 - thanks, checked it and works perfect.

marcinczeczko · 2020-05-06T11:03:59Z

@gsmet - I rebased with master to pick up #8994. So documentation generates correctly now, I think you can proceed with the final review now.

gsmet

@marcinczeczko I added some minor comments that needs fixing, could you take care of them and squash everything into a single commit?

There are a few things we need to discuss as I have a few questions but I suggest that we merge this one first and then we can have further discussions/improvements later on.

gsmet · 2020-05-06T12:42:19Z

extensions/amazon-services/dynamodb/runtime/pom.xml

    <artifactId>quarkus-amazon-dynamodb</artifactId>
-    <name>Quarkus - Amazon DynamoDB - Runtime</name>
-    <description>A client for the Amazon DynamoDB datastore</description>
+    <name>Quarkus - Amazon Services - DynamoDB - Runtime</name>


The description of the runtime modules is used by code.quarkus.io so we need one.

Better if it starts with a verb e.g. something like "Connect to...". Same for all the user facing extensions.

docs/src/main/asciidoc/amazon-s3.adoc

extensions/amazon-services/dynamodb/runtime/src/main/resources/META-INF/quarkus-extension.yaml

gsmet · 2020-05-06T13:55:35Z

Also, I saw we break the existing configuration properties. I suppose you find it better this way?

We can do that but we need to document it properly in the migration guide. We can do that later but prior to the release!

marcinczeczko · 2020-05-06T14:18:03Z

Also, I saw we break the existing configuration properties. I suppose you find it better this way?

We can do that but we need to document it properly in the migration guide. We can do that later but prior to the release!

Do you mean configurations that had .aws.. If yes, I think there were no a strong reason. Since the sdk config properties are not prefixed I simply decided to do so for aws onces. If you think it's better to keep as it was before I can easily change it.

gsmet · 2020-05-06T14:20:01Z

I dunno. The SDK ones were purely technical. The AWS ones were more like "let's configure the AWS part" so it kinda makes sense to me.

If there's no good reason, I would say we could keep them as is and avoid breaking user configuration?

- Refactored DynamoDB extension - Added S3 client extension - Bump up AWSSDK

marcinczeczko · 2020-05-06T14:44:23Z

I dunno. The SDK ones were purely technical. The AWS ones were more like "let's configure the AWS part" so it kinda makes sense to me.

If there's no good reason, I would say we could keep them as is and avoid breaking user configuration?

Agree & done

gsmet · 2020-05-06T15:10:47Z

OK, let's wait for CI and merge :).

Thanks @marcinczeczko for this great work and your patience and tenacity!

marcinczeczko · 2020-05-06T19:20:19Z

Thanks. :) So, next in a row are SQS and SNS extensions, so your patience will be required too. śr., 6 maj 2020, 17:11 użytkownik Guillaume Smet <[email protected]> napisał:

…

OK, let's wait for CI and merge :). Thanks @marcinczeczko <https://github.com/marcinczeczko> for this great work and your patience and tenacity! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4968 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABAF4G3MQBHH45KJPIGP2EDRQF4YRANCNFSM4JGFJ2YQ> .

sherl0cks · 2020-05-06T19:33:29Z

@marcinczeczko Thanks for making this happen!

oscerd · 2020-05-06T19:34:30Z

Thanks @marcinczeczko

gsmet · 2020-05-07T14:10:07Z

Cool, looking forward to it. Now that the basics are covered, it should be easier.

hrstoyanov · 2020-05-17T19:05:23Z

@marcinczeczko thanks for the effort! Any chance we can get SES (simple email service)

marcinczeczko · 2020-05-18T14:20:51Z

@hrstoyanov - thanks :) Actually I just opened PR #9413 with SES (and KMS)

marcinczeczko force-pushed the amazon-services branch from d9895e8 to adda9a3 Compare October 29, 2019 08:53

marcinczeczko mentioned this pull request Oct 29, 2019

Dynamodb - URL connection http client support #4471

Merged

marcinczeczko force-pushed the amazon-services branch from adda9a3 to 9b177c6 Compare October 29, 2019 09:58

marcinczeczko force-pushed the amazon-services branch from 9b177c6 to b4476f2 Compare October 31, 2019 08:46

marcinczeczko force-pushed the amazon-services branch from b4476f2 to ade329a Compare November 7, 2019 14:04

marcinczeczko force-pushed the amazon-services branch from ade329a to 904fa24 Compare November 28, 2019 10:49

gsmet requested changes Dec 4, 2019

View reviewed changes

oscerd approved these changes Dec 23, 2019

View reviewed changes

marcinczeczko force-pushed the amazon-services branch 2 times, most recently from 182d5ce to 952edd5 Compare January 13, 2020 09:56

machi1990 mentioned this pull request Jan 18, 2020

Refactor DynamoDB Extensions to Support Generic AWS SDK #6616

Closed

sherl0cks mentioned this pull request Jan 26, 2020

--initialize-at-build-time appears to be ignored | AWS XRay SDK #6785

Closed

marcinczeczko force-pushed the amazon-services branch 2 times, most recently from 660981f to 6c266a2 Compare February 25, 2020 09:37

gastaldi added the triage/needs-rebase This PR needs to be rebased first because it has merge conflicts label Mar 28, 2020

marcinczeczko force-pushed the amazon-services branch from 3b51dee to e242ce3 Compare May 3, 2020 19:59

marcinczeczko force-pushed the amazon-services branch from b999868 to 65cb687 Compare May 6, 2020 11:02

gsmet requested changes May 6, 2020

View reviewed changes

gsmet added release/breaking-change and removed triage/needs-rebase This PR needs to be rebased first because it has merge conflicts labels May 6, 2020

marcinczeczko force-pushed the amazon-services branch from dda8935 to 19ec2ce Compare May 6, 2020 14:19

Amazon service extensions

f1bfa9c

- Refactored DynamoDB extension - Added S3 client extension - Bump up AWSSDK

marcinczeczko force-pushed the amazon-services branch from 19ec2ce to f1bfa9c Compare May 6, 2020 14:44

marcinczeczko mentioned this pull request May 6, 2020

Amazon-services: change the path to dynamodb, added AWS S3 guide quarkusio/quarkusio.github.io#531

Merged

gsmet approved these changes May 6, 2020

View reviewed changes

gsmet removed the release/breaking-change label May 6, 2020

machi1990 merged commit 9a09b2f into quarkusio:master May 6, 2020

marcinczeczko mentioned this pull request May 7, 2020

S3 AWS Client does not work with quarkus in native mode #7846

Closed

gsmet added the release/noteworthy-feature label May 7, 2020

sherl0cks mentioned this pull request Oct 21, 2020

Amazon S3 Extension: Interceptors Configuration Fails to AWS Shipped Interceptors #12842

Closed

sherl0cks mentioned this pull request Nov 6, 2020

Make Amazon Services interceptors config a list of string #13105

Merged

drissamri mentioned this pull request Feb 7, 2021

Extend AWS DynamoDB extension for usage with Enhanced DynamoDb Client #12168

Open

Amazon service extensions #4968

Amazon service extensions #4968

Uh oh!

Conversation

marcinczeczko commented Oct 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gsmet commented Oct 29, 2019

Uh oh!

gsmet commented Nov 1, 2019

Uh oh!

marcinczeczko commented Nov 1, 2019

Uh oh!

marcinczeczko commented Nov 28, 2019

Uh oh!

gsmet commented Nov 28, 2019

Uh oh!

gsmet commented Dec 4, 2019

Uh oh!

gsmet left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gsmet commented Jan 6, 2020

Uh oh!

marcinczeczko commented Jan 7, 2020

Uh oh!

oztimpower commented Feb 24, 2020

Uh oh!

sherl0cks commented Apr 8, 2020

Uh oh!

oscerd commented Apr 10, 2020

Uh oh!

sherl0cks commented Apr 10, 2020

Uh oh!

oscerd commented Apr 10, 2020

Uh oh!

marcinczeczko commented Apr 10, 2020

Uh oh!

sherl0cks commented Apr 10, 2020

Uh oh!

marcinczeczko commented May 3, 2020

Uh oh!

marcinczeczko commented May 6, 2020

Uh oh!

gsmet left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

marcinczeczko commented Oct 29, 2019 •

edited

Loading