AWS EventBridge Support #2061

teroxik · 2019-12-27T15:53:15Z

Pull Request Checklist

Have you read through the contributor guidelines?
Have you signed the Lightbend CLA?
Have you updated the documentation?
Have you added tests for any changed functionality?

Purpose

based on the SNS implementation
adds Eventbridge Sink and Flow for publishing messages (PutEventsRequest / PutEventsRequestEntry) into the AWS EventBridge using the AWS 2.0 client

References

N/A

Changes

N/A

Background Context

N/A

seglo

AWS EventBridge looks like an interesting product. Thanks a lot for creating a PR! I did a first pass on the PR, but it looks like a great start.

Some general notes.

Consider the consequences of using mapAsyncUnordered with your concurrency setting. Is this important when using AWS EventBridge? Does the order of results ever matter?
More variety in the tests would be good to test error corner cases.
Alpakka documentation is still outstanding. We use paradox. I suggest using the SNS docs as a reference. Any worthwhile knowledge you've accumulated from using AWS EventBridge with Akka Streams should be added there. You can generate the docs locally with the sbt task docs/previewSite. I suggest opening an sbt session first and then make changes to the docs while you're in there, because there's a large startup performance penalty when calling the sbt task.

...bridge/src/main/scala/akka/stream/alpakka/aws/eventbridge/javadsl/EventBridgePublisher.scala

...ridge/src/main/scala/akka/stream/alpakka/aws/eventbridge/scaladsl/EventBridgePublisher.scala

seglo · 2020-01-06T16:37:47Z

...t-bridge/src/test/scala/akka/stream/alpakka/aws/eventbridge/EventBridgePublishMockSpec.scala

+    verify(eventBridgeClient, times(1)).putEvents(meq(expectedThird))
+  }
+
+  it should "publish multiple PublishRequest messages to multiple eventbridge topics" in {


It's not clear to me how this test is routing or asserting that messages are sent to different topics. It looks like the same impl as above.

...t-bridge/src/test/scala/akka/stream/alpakka/aws/eventbridge/EventBridgePublishMockSpec.scala

seglo · 2020-01-06T16:45:34Z

...t-bridge/src/test/scala/akka/stream/alpakka/aws/eventbridge/EventBridgePublishMockSpec.scala

+
+    verify(eventBridgeClient, never()).putEvents(any[PutEventsRequest]())
+  }
+


Maybe we could add a test that simulates an error returned by the service. There are some example error responses in the docs. https://docs.aws.amazon.com/eventbridge/latest/userguide/add-events-putevents.html

seglo · 2020-01-20T13:34:23Z

@teroxik Thanks for pushing this forward. I see you've labelled it as a WIP. Ping us when you're ready.

jmnarloch · 2020-02-20T04:32:12Z

Hey, I wish to +1 this change I think it will be really cool interop feature between Akka and EventBridge. I had done pretty much same code changes: #2151. Since you are already ahed of with the work I will close the other pull request.

Some thoughts that I have (some of them have been pointed out here)

Similarly as SQS EventBridge supports batching up to 10 events on PutEvents API. I don't feel it's essential to have in the first go, though eventually people might want to optimize they API calls.
I was experimenting with string message Sinks, which allows you to create them by providing the event bus name, source and detail type for the event, though I don't grasp how much this is useful and how often people are actually using plain strings. The only advantage that I see is that it removes a bit of boilerplate code that needs to be written to instantiate the PutEventsRequest.

Also if you think I can help on anything, let me know.

I can write a blog post once the change will be available as well.

teroxik · 2020-02-20T11:59:14Z

Hi there, I have the documentation almost ready. So will push it up this week. And we can hopefully move this forward and extend.

jmnarloch · 2020-03-18T05:36:56Z

@seglo any chance @teroxik PR would be reviewed?

teroxik · 2020-03-19T13:27:01Z

I have added the documentation. Still wanted to spent a bit more time on the test cases. Could do today.

seglo · 2020-03-23T16:15:06Z

@seglo any chance @teroxik PR would be reviewed?

Yes. @teroxik are you ready for another review? Could you take a look at the build? The checks failed.

teroxik · 2020-03-23T16:42:38Z

@seglo tomorrow if possible please. I will sort it.

seglo · 2020-03-23T17:10:32Z

Sure. There's no rush :)

teroxik · 2020-03-23T20:36:33Z

@seglo the MiMa check is failing as it's trying to find previous version of the module, but there is not one as it's new, any advice how to bypass that check?

teroxik · 2020-03-24T16:55:42Z

Will be looking into improving the test coverage as suggested today.

seglo · 2020-03-24T18:39:29Z

@seglo the MiMa check is failing as it's trying to find previous version of the module, but there is not one as it's new, any advice how to bypass that check?

Yes, for the new sbt project disable the MiMa plugin. Also, create a new alpakka issue to enable the MiMa plugin on this project once it's been released.

.disablePlugins(MimaPlugin)

seglo · 2020-03-24T18:40:03Z

Log the link to the new issue you create in a code comment where the sub project is defined.

mqtt-streaming/src/test/scala/docs/scaladsl/MqttActorSystemsSpec.scala

teroxik · 2020-03-25T19:57:33Z

build.sbt

@@ -280,6 +281,9 @@ lazy val simpleCodecs = alpakkaProject("simple-codecs", "simplecodecs")

 lazy val slick = alpakkaProject("slick", "slick", Dependencies.Slick)

+lazy val eventbridge =
+  alpakkaProject("aws-event-bridge", "aws.eventbridge", Dependencies.Eventbridge).disablePlugins(MimaPlugin)


Have to remove this disablePlugins after next release in order to check the Binary Compatibiliy.

jmnarloch · 2020-04-17T19:37:56Z

Any chance for another pass of review?

seglo · 2020-04-17T20:02:02Z

Yes, I meant to do a review this week but I kept getting distracted with other tasks. It's on my todo for next week. Sorry for the delay.

Doc updates from Seglo, thanks Co-authored-by: Sean Glover <sean@seanglover.com>

…o feature/event-bridge

teroxik · 2020-05-20T17:31:13Z

@seglo if you can help me out why the @apidoc is not working, correctly I have accepted the suggested updates for the comments - and that broke the build.


[error] Messages published in a batch using @apidoc[EventBridgePublisher.flowSeq] are not published in an "all or nothing" manner. Event Bridge will process each event independently. Retries of the failed messages in the `PutEventsResponse` are not yet implemented.```

seglo · 2020-05-22T16:48:56Z

@teroxik Thanks. I've pushed a fix. I'll follow up on Monday with another review.

seglo

LGTM

seglo · 2020-05-26T18:52:31Z

Thanks a lot for this contribution @teroxik! It will be published to the akka snapshot repo next successful build of master.

teroxik · 2020-05-26T20:17:11Z

Hi there, I think there is some more followup work still required but let's take it from there. Thanks for the help.

seglo · 2020-05-27T13:33:14Z

@teroxik Is there anything required that you think should gate the first release? We should create GitHub issues to address any outstanding work.

WDYT @jmnarloch?

teroxik · 2020-06-03T16:48:13Z

@seglo I don't think there are any issues, mainly some additional suggestion we have discussed - in terms of testing, adding different shape for the sinks and flow API, based on how people would prefer to use it.

probot-autolabeler bot added the dependency-change For PRs changing the version of a dependency. label Dec 27, 2019

seglo reviewed Jan 6, 2020

View reviewed changes

teroxik changed the title ~~AWS EventBridge Support~~ [WIP] AWS EventBridge Support Jan 19, 2020

teroxik force-pushed the feature/event-bridge branch from 75f9e0c to 5e34e59 Compare February 16, 2020 22:49

seglo mentioned this pull request Feb 18, 2020

Add support for AWS EventBridge #2151

Closed

probot-autolabeler bot added the documentation label Feb 23, 2020

seglo mentioned this pull request Mar 3, 2020

Support AWS EventBridge (#2151) #2152

Closed

probot-autolabeler bot added the p:mqtt-streaming label Mar 23, 2020

seglo added p:new and removed p:mqtt-streaming labels Mar 24, 2020

probot-autolabeler bot added the p:mqtt-streaming label Mar 25, 2020

seglo reviewed Mar 25, 2020

View reviewed changes

mqtt-streaming/src/test/scala/docs/scaladsl/MqttActorSystemsSpec.scala Show resolved Hide resolved

seglo removed the p:mqtt-streaming label Mar 25, 2020

teroxik commented Mar 25, 2020

View reviewed changes

teroxik changed the title ~~[WIP] AWS EventBridge Support~~ AWS EventBridge Support Apr 21, 2020

teroxik requested a review from seglo April 21, 2020 18:24

teroxik added 13 commits May 19, 2020 20:24

Documentation and updates

4441bb3

Logging and Windows metals.sbt

68b61c0

Fix tests, format, version bumps

8e3d2e9

Formating

cf126fa

Fix documentatnion link

0af5a3d

Fix paradox

704f84a

Configuration

cdc4488

Update of the documentation

5d99462

Disable MIMA

5bd16fb

Remove change

2a3a688

Replace eventbus name generation logic

af687e6

Remove unused import

7446eb2

Update tests / docs

32eb154

teroxik force-pushed the feature/event-bridge branch from 79a9dd1 to 32eb154 Compare May 19, 2020 19:25

probot-autolabeler bot added the p:mqtt-streaming label May 19, 2020

teroxik and others added 3 commits May 19, 2020 22:14

Apply suggestions from code review

95e7989

Doc updates from Seglo, thanks Co-authored-by: Sean Glover <sean@seanglover.com>

Update of id bus generation clash

2e02dc3

Merge branch 'feature/event-bridge' of github.com:teroxik/alpakka int…

f37d228

…o feature/event-bridge

apidocs fix

acaf0ed

minor cleanup

b2d769e

seglo approved these changes May 26, 2020

View reviewed changes

seglo merged commit 5f012f7 into akka:master May 26, 2020

ennru added p:aws-event-bridge and removed p:mqtt-streaming labels Jun 9, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AWS EventBridge Support #2061

AWS EventBridge Support #2061

teroxik commented Dec 27, 2019 •

edited

Loading

seglo left a comment

seglo Jan 6, 2020

seglo Jan 6, 2020

seglo commented Jan 20, 2020

jmnarloch commented Feb 20, 2020 •

edited

Loading

teroxik commented Feb 20, 2020 •

edited

Loading

jmnarloch commented Mar 18, 2020

teroxik commented Mar 19, 2020

seglo commented Mar 23, 2020

teroxik commented Mar 23, 2020

seglo commented Mar 23, 2020

teroxik commented Mar 23, 2020

teroxik commented Mar 24, 2020

seglo commented Mar 24, 2020

seglo commented Mar 24, 2020

teroxik Mar 25, 2020

jmnarloch commented Apr 17, 2020

seglo commented Apr 17, 2020

teroxik commented May 20, 2020

seglo commented May 22, 2020

seglo left a comment

seglo commented May 26, 2020

teroxik commented May 26, 2020

seglo commented May 27, 2020

teroxik commented Jun 3, 2020


		verify(eventBridgeClient, never()).putEvents(any[PutEventsRequest]())
		}

AWS EventBridge Support #2061

AWS EventBridge Support #2061

Conversation

teroxik commented Dec 27, 2019 • edited Loading

Pull Request Checklist

Purpose

References

Changes

Background Context

seglo left a comment

Choose a reason for hiding this comment

seglo Jan 6, 2020

Choose a reason for hiding this comment

seglo Jan 6, 2020

Choose a reason for hiding this comment

seglo commented Jan 20, 2020

jmnarloch commented Feb 20, 2020 • edited Loading

teroxik commented Feb 20, 2020 • edited Loading

jmnarloch commented Mar 18, 2020

teroxik commented Mar 19, 2020

seglo commented Mar 23, 2020

teroxik commented Mar 23, 2020

seglo commented Mar 23, 2020

teroxik commented Mar 23, 2020

teroxik commented Mar 24, 2020

seglo commented Mar 24, 2020

seglo commented Mar 24, 2020

teroxik Mar 25, 2020

Choose a reason for hiding this comment

jmnarloch commented Apr 17, 2020

seglo commented Apr 17, 2020

teroxik commented May 20, 2020

seglo commented May 22, 2020

seglo left a comment

Choose a reason for hiding this comment

seglo commented May 26, 2020

teroxik commented May 26, 2020

seglo commented May 27, 2020

teroxik commented Jun 3, 2020

teroxik commented Dec 27, 2019 •

edited

Loading

jmnarloch commented Feb 20, 2020 •

edited

Loading

teroxik commented Feb 20, 2020 •

edited

Loading