[ISSUE #199] Kafka Connector: Initial Implementation of Consumer & Producer #1014
Conversation
| //import org.apache.eventmesh.api.exception.ConnectorRuntimeException; | ||
| //import org.apache.eventmesh.api.exception.OnExceptionContext; | ||
| //import org.apache.eventmesh.api.producer.Producer; | ||
| //import org.apache.eventmesh.common.Constants; |
There was a problem hiding this comment.
Please remove the invalid code.
Markliniubility
changed the title
Codecov Report
@@ 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
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
|
License check did not pass, please check the license format or did you forget to add a license? |
|
Hello, please fix the license check problem. |
|
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. |
| # under the License. | ||
| # | ||
|
|
||
| org.apache.eventmesh.connector.kafka.producer.ProducerImpl |
There was a problem hiding this comment.
Hi, these test resource files is never used. Maybe you should commit them when you really need.
| loadProperties(); | ||
| } | ||
|
|
||
| public String getProp(String key) { |
There was a problem hiding this comment.
This method is never used. Maybe you need write another Configration class like RocketMQ Connector to enable your .properties file.
| Map<String, List<PartitionInfo>> topicsAndPartition = this.kafkaConsumer.listTopics(); | ||
| Set<String> topicsSet = topicsAndPartition.keySet(); | ||
| List<String> topics = new ArrayList<>(); | ||
| topics.addAll(topicsSet); |
There was a problem hiding this comment.
Hi, you get all topics but not do any change of them. It seems that you should remove the special topic from the already subscribed topics.
Markliniubility
changed the title
Markliniubility
changed the title
Markliniubility
changed the title
xwm1992
changed the title
[ISSUE #199] Kafka Connector: Initial Implementation of Consumer & Producer
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