eventmesh icon indicating copy to clipboard operation
eventmesh copied to clipboard

[ISSUE #199] Kafka Connector: Initial Implementation of Consumer & Producer

Open Markliniubility opened this issue 3 years ago • 5 comments
trafficstars

Impl ISSUE #199

Motivation

The goal is to expand the event storage component that EventMesh can support, Kafka, based on the existing API. This PR is the first step of this project, which implemented a consumer.

This PR, as well as all the following PR on Kafka Connectors, will not be merged into the master. It will be merged into a Kafka branch instead

Modifications

A basic/mimimal consumer of Kafka.

Documentation

  • Does this pull request introduces a new feature? (yes / no)
  • Yes
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
  • Not yet documented
  • If a feature is not applicable for documentation, explain why?
  • It will be documented after the completion of the producer and a functional Kafka connector
  • If a feature is not documented yet in this PR, please create a follow-up issue for adding the documentation
  • The follow-up will be creating the producer, as well as implementing more features of Kafka

Markliniubility avatar Jul 12 '22 03:07 Markliniubility

Codecov Report

Merging #1014 (6b53e20) into kafka-connector (dfe58f5) will increase coverage by 0.01%. The diff coverage is 0.00%.

@@                 Coverage Diff                  @@
##             kafka-connector   #1014      +/-   ##
====================================================
+ Coverage               8.37%   8.39%   +0.01%     
  Complexity               529     529              
====================================================
  Files                    362     362              
  Lines                  23228   23245      +17     
  Branches                2546    2546              
====================================================
+ Hits                    1946    1951       +5     
- Misses                 21132   21144      +12     
  Partials                 150     150              
Impacted Files Coverage Δ
.../connector/rocketmq/consumer/PushConsumerImpl.java 24.64% <ø> (ø)
...nector/rocketmq/producer/RocketMQProducerImpl.java 0.00% <0.00%> (ø)
...pl/eventmeshmessage/EventMeshMessageTCPClient.java 10.81% <0.00%> (-2.99%) :arrow_down:
...pache/eventmesh/runtime/boot/EventMeshStartup.java 0.00% <0.00%> (ø)
.../http/processor/RemoteSubscribeEventProcessor.java 0.00% <0.00%> (ø)
.../http/processor/SendAsyncRemoteEventProcessor.java 0.00% <0.00%> (ø)
.../eventmesh/common/config/ConfigurationWrapper.java 50.00% <0.00%> (+6.75%) :arrow_up:

:mega: Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

codecov[bot] avatar Jul 12 '22 09:07 codecov[bot]

License check did not pass, please check the license format or did you forget to add a license?

Alonexc avatar Jul 13 '22 01:07 Alonexc

Hello, please fix the license check problem.

mytang0 avatar Jul 13 '22 02:07 mytang0

Hi, thanks for your initial implementation. Here are some suggestions.

This pr just implements consumer without enough tests. I recommend you implement producer first and verify it through local kafka server. If you could write some UT with kafka testcontainer, it will be much better. After that, we could have a base to implement consumer correctly.

What's more, please replace rocketmq with kafka in your code and comment.

LIU-WEI-git avatar Jul 25 '22 09:07 LIU-WEI-git

Hi, thanks for your initial implementation. Here are some suggestions.

This pr just implements consumer without enough tests. I recommend you implement producer first and verify it through local kafka server. If you could write some UT with kafka testcontainer, it will be much better. After that, we could have a base to implement consumer correctly.

What's more, please replace rocketmq with kafka in your code and comment.

Thank you for reviewing! I agree with you on implementing the producer first (which I should have done first) and providing unit tests, and I am working on implementing the producer. However, the branch was only going to merge into a development branch, kafka-connector, instead of the master branch. I was going to add the unit tests and producer in separate PRs and make sure everything works and is fully tested before rolling out to the master branch. I prefer splitting the project into small PRs since it can be more easily reviewed and understood.

Markliniubility avatar Jul 25 '22 11:07 Markliniubility