dashbitco / broadway_kafka

A Broadway connector for Kafka
233 stars 53 forks source link

feat: implement shared client #133

Closed oliveigah closed 1 year ago

oliveigah commented 1 year ago

Context: https://github.com/dashbitco/broadway_kafka/issues/132

First attempt to solve the related issue. Let me know what you think! :smile:

Some considerations:

oliveigah commented 1 year ago

Applied the proposed changes @josevalim!

josevalim commented 1 year ago

Tests are failing, can you please take a look? The linting one is quite old, so feel free to bump the Elixir version to make the formatter happier.

oliveigah commented 1 year ago

Tests are failing, can you please take a look?

Sure! I've tested everything locally and everything passed.

Gonna try to reproduce it. Maybe it is flaky somehow.

oliveigah commented 1 year ago

@josevalim, I could not reproduce the integration test problem neither locally or on CI.

But my guess is that using the same topic/group might have impacted in some way, also I increased the waiting time of the integration test, since on the failed run we missed only 1 message (999 expected 1000).

So in summary the changes were:

josevalim commented 1 year ago

:green_heart: :blue_heart: :purple_heart: :yellow_heart: :heart: