Skip to content
This repository has been archived by the owner on Oct 23, 2023. It is now read-only.

Fix #25 by providing an Akka Stream Source graph stage for Kinesis. #36

Closed
wants to merge 2 commits into from

Conversation

aquamatthias
Copy link
Contributor

This graph stage provides an akka source graph based on the actor model provided by the reactive kinesis consumer.

If a consumer is configured, an akka stream Source can be obtained via a simple Kinesis.source("consumer-name").
Every message that flows through the stream needs to be committed explicitly.

An integration test is added that relies on localstack running on the same node (I did not add the localstack startup logic as part of this PR).

Steps to run the test:

  1. Set this env var, since the localstack does not support CBOR: export AWS_CBOR_DISABLE=true
  2. Start localstack (I use the docker-compose with: docker-compose -f localstack.yml up) in default configuration (Kinesis on port 4568, Dynamo on port 4569)
  3. sbt it:test

It was not as easy to come up with a good integration test, since a Kinesis Source usually never finishes. To have a finite test, I use Flow.take which ends the stream after a defined amount of entries. The interplay of different readers/shard consumers could be different between runs, but the test expectation should be met.

…Kinesis.

This graph stage provides an akka source graph based on the actor model provided by the reactive kinesis consumer.
@coveralls
Copy link

coveralls commented Oct 28, 2017

Coverage Status

Coverage decreased (-6.008%) to 80.769% when pulling f357797 on aquamatthias:mv/akka_stream into 5bff161 on WW-Digital:master.

@markglh
Copy link
Contributor

markglh commented Oct 28, 2017

Thanks @aquamatthias! Regarding the localstack stuff - I'm in the process of adding that and hooking it up to travis in #34

I've got a lot of travelling to do this week but I'll dig into this properly over the next couple of weeks 👌

@calvinlfer
Copy link

This is really cool!

@markglh
Copy link
Contributor

markglh commented Nov 29, 2017

Ok it's been a month - so sorry @aquamatthias - now things have settled down it's high up on my list!!

@markglh
Copy link
Contributor

markglh commented Dec 3, 2017

I've finished the integration tests finally - so they run in travis now against localstack.

This is next...

@markglh
Copy link
Contributor

markglh commented Dec 3, 2017

Created a branch off this to rebase master and refactor the integration tests. My intention is to find a middle ground between what I did and what you did. There are pros and cons of each!

https://github.com/WW-Digital/reactive-kinesis/tree/feature/aquamatthias/akka-streams

Right now it's building and running in travis - but not quite finished yet.

Not spent much time looking at the main code yet / reviewing...

Seen this fail a couple of times in the travis builds:

[info] - should maintain the read position in the stream correctly *** FAILED ***
[info]   Vector("1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "15", "16", "17", "18", "19", "20", "21", "22", "23", "24", "25", "26", "27", "28", "29", "30", "31", "32", "33", "34", "35", "36", "37", "38", "39", "40", "41", "42", "43", "44", "45", "46", "47", "48", "49", "50", "51", "52", "53", "54", "55", "56", "57", "58", "59", "60", "61", "62", "63", "64", "65", "66", "67", "68", "69", "70", "71", "72", "73", "74", "75", "76", "77", "78", "79", "80", "81", "82", "83", "84", "85", "86", "87", "88", "89", "90", "91", "92", "93", "94", "95", "96", "97", "98", "99") had size 99 instead of expected size 100 (KinesisSourceSpec.scala:135)

May be my refactoring - I'll keep prodding :)

@aquamatthias
Copy link
Contributor Author

@markglh Sounds good. Let me know if I there is something I can help with.
The failed test sounds like a shaky test - I will try to investigate.

@markglh
Copy link
Contributor

markglh commented Dec 4, 2017

Thanks @aquamatthias - you should be able to merge my branch into yours without conflicts 👌

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants