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

feat: Kafka Consumer Package #342

Merged
merged 71 commits into from Apr 2, 2021
Merged

Conversation

oss92
Copy link

@oss92 oss92 commented Feb 4, 2021

Which problem is this PR solving?

Closes #260

Short description of the changes

We've had a few discussions about the best way to move forward with the task; the key outcomes were

  • Kafka's streaming logic is different than the message broker logic of AMQP and SNS/SQS. Trying to contain both under a single abstraction is not ideal.
  • Instead of maintaining the current abstraction, we've decided to introduce a new Kafka package with support for batching and
  • This new package is more transparent in regards to Sarama; exposing more its internals, plus with QoL helpers for the end-users.

Checklist

  • Component
  • Retries
  • Batching
  • Raw Sarama Message
  • Raw Sarama Config
  • Update Sarama
  • Failure strategy
  • Metrics
  • Tracing
  • Unit tests
  • Integration tests

@oss92 oss92 changed the title Kafka raw consumer Kafka Consumer Package Feb 4, 2021
@codecov
Copy link

codecov bot commented Feb 4, 2021

Codecov Report

Merging #342 (b42e1f6) into master (7f52baf) will increase coverage by 0.84%.
The diff coverage is 86.69%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #342      +/-   ##
==========================================
+ Coverage   72.66%   73.50%   +0.84%     
==========================================
  Files          71       74       +3     
  Lines        3859     4077     +218     
==========================================
+ Hits         2804     2997     +193     
- Misses        941      963      +22     
- Partials      114      117       +3     
Impacted Files Coverage Δ
component/kafka/component.go 84.74% <84.74%> (ø)
component/kafka/kafka.go 86.66% <86.66%> (ø)
component/kafka/option.go 100.00% <100.00%> (ø)
component/sqs/component.go 87.77% <0.00%> (+1.11%) ⬆️
component/async/kafka/simple/duration_client.go 96.49% <0.00%> (+3.50%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7f52baf...2c177ba. Read the comment docs.

@oss92 oss92 force-pushed the kafka-raw-consumer branch 19 times, most recently from c59249b to ee09855 Compare February 10, 2021 13:20
@mantzas mantzas self-requested a review March 29, 2021 04:41
mantzas
mantzas previously approved these changes Mar 29, 2021
Copy link

@mantzas mantzas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I am eager to start to use it. Very nice work.

mantzas
mantzas previously approved these changes Mar 29, 2021
gmaz42
gmaz42 previously approved these changes Mar 29, 2021
Copy link

@gmaz42 gmaz42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is solid work, thanks! Looking forward to use it ☺️

I tried to think how to make some loops simpler but could not think of anything better.

🥇

@oss92 oss92 dismissed stale reviews from gmaz42 and mantzas via c90d1b3 March 29, 2021 13:07
gmaz42
gmaz42 previously approved these changes Mar 29, 2021
Copy link

@tpaschalis tpaschalis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢 nice work! Looking forward to toying with it!

@mantzas mantzas merged commit 8e1f756 into beatlabs:master Apr 2, 2021
@oss92 oss92 deleted the kafka-raw-consumer branch April 2, 2021 15:28
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.

Kafka Batch Consumer
5 participants