You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@eventmesh.apache.org by "pandaapo (via GitHub)" <gi...@apache.org> on 2023/05/04 21:14:17 UTC
[GitHub] [eventmesh] pandaapo opened a new pull request, #3867: [ISSUE #3861]Not yet make full use of Polymorphism in Producer and AbstractProducer.
pandaapo opened a new pull request, #3867:
URL: https://github.com/apache/eventmesh/pull/3867
Fixes #3861.
### Motivation
See "Enhancement Request" in [issue](https://github.com/apache/eventmesh/issues/3861#issue-1694155912)
### Modifications
See "Describe the solution you'd like" in [issue](https://github.com/apache/eventmesh/issues/3861#issue-1694155912)
### Documentation
- Does this pull request introduce a new feature? no
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
Re: [PR] [ISSUE #3861] Not yet make full use of Polymorphism in Producer and AbstractProducer (eventmesh)
Posted by "pandaapo (via GitHub)" <gi...@apache.org>.
pandaapo commented on code in PR #3867:
URL: https://github.com/apache/eventmesh/pull/3867#discussion_r1576133454
##########
eventmesh-storage-plugin/eventmesh-storage-kafka/src/main/java/org/apache/eventmesh/storage/kafka/producer/ProducerImpl.java:
##########
@@ -42,36 +41,20 @@
@Slf4j
@SuppressWarnings("deprecation")
-public class ProducerImpl {
+public class ProducerImpl extends AbstractProducer {
private final KafkaProducer<String, CloudEvent> producer;
- private final Properties properties = new Properties();
- private final AtomicBoolean isStarted = new AtomicBoolean(false);
public ProducerImpl(Properties props) {
+ super(props);
+ properties.clear();
Review Comment:
Actually, when we read the code of `ProducerImpl` and `KafkaProducerImpl` , it is quite evident that the latter is merely a delegate class, delegating all the specific details to former for actual processing. Of course, you can use the code from the corresponding classes in RocketMQ module to refute me. However, I personally favor the positioning of these two classes in Kafka module, as their responsibilities are more clearly defined.
Finally Community Over Code. I'm willing to wait for more members in community to express their views on this point. If the approach used in RocketMQ module is more accepted, I will revise.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #3861] Not yet make full use of Polymorphism in Producer and AbstractProducer (eventmesh)
Posted by "pandaapo (via GitHub)" <gi...@apache.org>.
pandaapo commented on code in PR #3867:
URL: https://github.com/apache/eventmesh/pull/3867#discussion_r1577011228
##########
eventmesh-storage-plugin/eventmesh-storage-kafka/src/main/java/org/apache/eventmesh/storage/kafka/producer/ProducerImpl.java:
##########
@@ -42,36 +41,20 @@
@Slf4j
@SuppressWarnings("deprecation")
-public class ProducerImpl {
+public class ProducerImpl extends AbstractProducer {
private final KafkaProducer<String, CloudEvent> producer;
- private final Properties properties = new Properties();
- private final AtomicBoolean isStarted = new AtomicBoolean(false);
public ProducerImpl(Properties props) {
+ super(props);
+ properties.clear();
Review Comment:
> I agree with your view. Since the abstraction has already been made, how about standardizing the usage of the abstract implementation?
My view on what you call 'standardizing' is similar to https://github.com/apache/eventmesh/pull/3867#discussion_r1576133454 above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
[GitHub] [eventmesh] pandaapo commented on a diff in pull request #3867: [ISSUE #3861]Not yet make full use of Polymorphism in Producer and AbstractProducer.
Posted by "pandaapo (via GitHub)" <gi...@apache.org>.
pandaapo commented on code in PR #3867:
URL: https://github.com/apache/eventmesh/pull/3867#discussion_r1193975690
##########
eventmesh-storage-plugin/eventmesh-storage-kafka/src/main/java/org/apache/eventmesh/storage/kafka/producer/ProducerImpl.java:
##########
@@ -42,36 +41,20 @@
@Slf4j
@SuppressWarnings("deprecation")
-public class ProducerImpl {
+public class ProducerImpl extends AbstractProducer {
private final KafkaProducer<String, CloudEvent> producer;
- private final Properties properties = new Properties();
- private final AtomicBoolean isStarted = new AtomicBoolean(false);
public ProducerImpl(Properties props) {
+ super(props);
+ properties.clear();
Review Comment:
Because `super(props)` assigns param `props` to field `properties`. but `ProducerImpl` does not assign value like that. What it do to `properties` is just the next 3 line.
https://github.com/apache/eventmesh/blob/470329d7e3c69fa241762b364f1cd1799a3b5766/eventmesh-storage-plugin/eventmesh-storage-kafka/src/main/java/org/apache/eventmesh/storage/kafka/producer/ProducerImpl.java#L51-L54
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #3861]Not yet make full use of Polymorphism in Producer and AbstractProducer. (eventmesh)
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3867:
URL: https://github.com/apache/eventmesh/pull/3867#issuecomment-2070568567
It has been 60 days since the last activity on this pull request. I am reaching out here to gently remind you that the Apache EventMesh community values every pull request, and please feel free to get in touch with the reviewers at any time. They are available to assist you in advancing the progress of your pull request and offering the latest feedback.
If you encounter any challenges during development, seeking support within the community is encouraged. We sincerely appreciate your contributions to Apache EventMesh.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #3861] Not yet make full use of Polymorphism in Producer and AbstractProducer (eventmesh)
Posted by "pandaapo (via GitHub)" <gi...@apache.org>.
pandaapo commented on code in PR #3867:
URL: https://github.com/apache/eventmesh/pull/3867#discussion_r1577011228
##########
eventmesh-storage-plugin/eventmesh-storage-kafka/src/main/java/org/apache/eventmesh/storage/kafka/producer/ProducerImpl.java:
##########
@@ -42,36 +41,20 @@
@Slf4j
@SuppressWarnings("deprecation")
-public class ProducerImpl {
+public class ProducerImpl extends AbstractProducer {
private final KafkaProducer<String, CloudEvent> producer;
- private final Properties properties = new Properties();
- private final AtomicBoolean isStarted = new AtomicBoolean(false);
public ProducerImpl(Properties props) {
+ super(props);
+ properties.clear();
Review Comment:
My view on what you call 'standardizing' is similar to https://github.com/apache/eventmesh/pull/3867#discussion_r1576133454 above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
[GitHub] [eventmesh] xwm1992 commented on a diff in pull request #3867: [ISSUE #3861]Not yet make full use of Polymorphism in Producer and AbstractProducer.
Posted by "xwm1992 (via GitHub)" <gi...@apache.org>.
xwm1992 commented on code in PR #3867:
URL: https://github.com/apache/eventmesh/pull/3867#discussion_r1193766729
##########
eventmesh-storage-plugin/eventmesh-storage-kafka/src/main/java/org/apache/eventmesh/storage/kafka/producer/ProducerImpl.java:
##########
@@ -42,36 +41,20 @@
@Slf4j
@SuppressWarnings("deprecation")
-public class ProducerImpl {
+public class ProducerImpl extends AbstractProducer {
private final KafkaProducer<String, CloudEvent> producer;
- private final Properties properties = new Properties();
- private final AtomicBoolean isStarted = new AtomicBoolean(false);
public ProducerImpl(Properties props) {
+ super(props);
+ properties.clear();
Review Comment:
why use `clear()` method at here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
[GitHub] [eventmesh] mroccyen commented on pull request #3867: [ISSUE #3861]Not yet make full use of Polymorphism in Producer and AbstractProducer.
Posted by "mroccyen (via GitHub)" <gi...@apache.org>.
mroccyen commented on PR #3867:
URL: https://github.com/apache/eventmesh/pull/3867#issuecomment-1539594690
@pandaapo Please fixes conflict.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
[GitHub] [eventmesh] codecov[bot] commented on pull request #3867: [ISSUE #3861]Not yet make full use of Polymorphism in Producer and AbstractProducer.
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #3867:
URL: https://github.com/apache/eventmesh/pull/3867#issuecomment-1535830553
## [Codecov](https://app.codecov.io/gh/apache/eventmesh/pull/3867?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#3867](https://app.codecov.io/gh/apache/eventmesh/pull/3867?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b15744f) into [master](https://app.codecov.io/gh/apache/eventmesh/commit/c5b7cf44e526f01016e2f05af7fde3e7c0534462?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c5b7cf4) will **increase** coverage by `0.00%`.
> The diff coverage is `38.46%`.
> :exclamation: Current head b15744f differs from pull request most recent head f938a9c. Consider uploading reports for the commit f938a9c to get more accurate results
```diff
@@ Coverage Diff @@
## master #3867 +/- ##
=========================================
Coverage 13.81% 13.81%
+ Complexity 1291 1289 -2
=========================================
Files 571 570 -1
Lines 29090 29043 -47
Branches 2841 2829 -12
=========================================
- Hits 4019 4013 -6
+ Misses 24698 24659 -39
+ Partials 373 371 -2
```
| [Impacted Files](https://app.codecov.io/gh/apache/eventmesh/pull/3867?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...eventmesh/storage/kafka/producer/ProducerImpl.java](https://app.codecov.io/gh/apache/eventmesh/pull/3867?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLXN0b3JhZ2UtcGx1Z2luL2V2ZW50bWVzaC1zdG9yYWdlLWthZmthL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9ldmVudG1lc2gvc3RvcmFnZS9rYWZrYS9wcm9kdWNlci9Qcm9kdWNlckltcGwuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ventmesh/storage/pulsar/producer/ProducerImpl.java](https://app.codecov.io/gh/apache/eventmesh/pull/3867?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLXN0b3JhZ2UtcGx1Z2luL2V2ZW50bWVzaC1zdG9yYWdlLXB1bHNhci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZXZlbnRtZXNoL3N0b3JhZ2UvcHVsc2FyL3Byb2R1Y2VyL1Byb2R1Y2VySW1wbC5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ntmesh/storage/rocketmq/producer/ProducerImpl.java](https://app.codecov.io/gh/apache/eventmesh/pull/3867?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLXN0b3JhZ2UtcGx1Z2luL2V2ZW50bWVzaC1zdG9yYWdlLXJvY2tldG1xL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9ldmVudG1lc2gvc3RvcmFnZS9yb2NrZXRtcS9wcm9kdWNlci9Qcm9kdWNlckltcGwuamF2YQ==) | `22.10% <16.66%> (+0.45%)` | :arrow_up: |
| [...entmesh/storage/knative/producer/ProducerImpl.java](https://app.codecov.io/gh/apache/eventmesh/pull/3867?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLXN0b3JhZ2UtcGx1Z2luL2V2ZW50bWVzaC1zdG9yYWdlLWtuYXRpdmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9zdG9yYWdlL2tuYXRpdmUvcHJvZHVjZXIvUHJvZHVjZXJJbXBsLmphdmE=) | `59.45% <25.00%> (+8.23%)` | :arrow_up: |
| [...ge/rocketmq/producer/RocketMQAbstractProducer.java](https://app.codecov.io/gh/apache/eventmesh/pull/3867?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLXN0b3JhZ2UtcGx1Z2luL2V2ZW50bWVzaC1zdG9yYWdlLXJvY2tldG1xL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9ldmVudG1lc2gvc3RvcmFnZS9yb2NrZXRtcS9wcm9kdWNlci9Sb2NrZXRNUUFic3RyYWN0UHJvZHVjZXIuamF2YQ==) | `38.23% <63.63%> (ø)` | |
| [...rage/knative/producer/KnativeAbstractProducer.java](https://app.codecov.io/gh/apache/eventmesh/pull/3867?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLXN0b3JhZ2UtcGx1Z2luL2V2ZW50bWVzaC1zdG9yYWdlLWtuYXRpdmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9zdG9yYWdlL2tuYXRpdmUvcHJvZHVjZXIvS25hdGl2ZUFic3RyYWN0UHJvZHVjZXIuamF2YQ==) | `100.00% <100.00%> (ø)` | |
... and [7 files with indirect coverage changes](https://app.codecov.io/gh/apache/eventmesh/pull/3867/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #3861] Not yet make full use of Polymorphism in Producer and AbstractProducer (eventmesh)
Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on code in PR #3867:
URL: https://github.com/apache/eventmesh/pull/3867#discussion_r1576493947
##########
eventmesh-storage-plugin/eventmesh-storage-kafka/src/main/java/org/apache/eventmesh/storage/kafka/producer/ProducerImpl.java:
##########
@@ -42,36 +41,20 @@
@Slf4j
@SuppressWarnings("deprecation")
-public class ProducerImpl {
+public class ProducerImpl extends AbstractProducer {
private final KafkaProducer<String, CloudEvent> producer;
- private final Properties properties = new Properties();
- private final AtomicBoolean isStarted = new AtomicBoolean(false);
public ProducerImpl(Properties props) {
+ super(props);
+ properties.clear();
Review Comment:
>it is quite evident that the latter is merely a delegate class, delegating all the specific details to former for actual processing
I agree with your view. Since the abstraction has already been made, how about standardizing the usage of the abstract implementation?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #3861] Not yet make full use of Polymorphism in Producer and AbstractProducer (eventmesh)
Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on code in PR #3867:
URL: https://github.com/apache/eventmesh/pull/3867#discussion_r1575643937
##########
eventmesh-storage-plugin/eventmesh-storage-kafka/src/main/java/org/apache/eventmesh/storage/kafka/producer/ProducerImpl.java:
##########
@@ -42,36 +41,20 @@
@Slf4j
@SuppressWarnings("deprecation")
-public class ProducerImpl {
+public class ProducerImpl extends AbstractProducer {
private final KafkaProducer<String, CloudEvent> producer;
- private final Properties properties = new Properties();
- private final AtomicBoolean isStarted = new AtomicBoolean(false);
public ProducerImpl(Properties props) {
+ super(props);
+ properties.clear();
Review Comment:
Excuse me but I still don't understand that why Kafka needs a `clear()` statement here but RocketMQ and Pulsar don't.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #3861] Not yet make full use of Polymorphism in Producer and AbstractProducer (eventmesh)
Posted by "pandaapo (via GitHub)" <gi...@apache.org>.
pandaapo commented on code in PR #3867:
URL: https://github.com/apache/eventmesh/pull/3867#discussion_r1575956640
##########
eventmesh-storage-plugin/eventmesh-storage-kafka/src/main/java/org/apache/eventmesh/storage/kafka/producer/ProducerImpl.java:
##########
@@ -42,36 +41,20 @@
@Slf4j
@SuppressWarnings("deprecation")
-public class ProducerImpl {
+public class ProducerImpl extends AbstractProducer {
private final KafkaProducer<String, CloudEvent> producer;
- private final Properties properties = new Properties();
- private final AtomicBoolean isStarted = new AtomicBoolean(false);
public ProducerImpl(Properties props) {
+ super(props);
+ properties.clear();
Review Comment:
Then I explain it in detail again.
The following is the original logic of this method under Kafka module. You can see that only three items need to be set in `properties` object. Now after the unified abstraction, `properties` is first assigned a value, which is equivalent to setting more items. If I don't `clear()` first, it firstly disrupts the original logic, secondly causes data contamination, and thirdly, the additional unknown items set might lead to unnecessary and unknown problems.
https://github.com/apache/eventmesh/blob/9ce5dd6fd94d60d47358721a18eff0f50cd932ad/eventmesh-storage-plugin/eventmesh-storage-kafka/src/main/java/org/apache/eventmesh/storage/kafka/producer/ProducerImpl.java#L51-L56
As for why RocketMQ and Pulsar modules don't need `clear()`, because it is consistent with their original logic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #3861] Not yet make full use of Polymorphism in Producer and AbstractProducer (eventmesh)
Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on code in PR #3867:
URL: https://github.com/apache/eventmesh/pull/3867#discussion_r1576050361
##########
eventmesh-storage-plugin/eventmesh-storage-kafka/src/main/java/org/apache/eventmesh/storage/kafka/producer/ProducerImpl.java:
##########
@@ -42,36 +41,20 @@
@Slf4j
@SuppressWarnings("deprecation")
-public class ProducerImpl {
+public class ProducerImpl extends AbstractProducer {
private final KafkaProducer<String, CloudEvent> producer;
- private final Properties properties = new Properties();
- private final AtomicBoolean isStarted = new AtomicBoolean(false);
public ProducerImpl(Properties props) {
+ super(props);
+ properties.clear();
Review Comment:
Then how about moving the `properties.put` statements of `org.apache.eventmesh.storage.kafka.producer.ProducerImpl` into the `init()` method of `org.apache.eventmesh.storage.kafka.producer.KafkaProducerImpl`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #3861]Not yet make full use of Polymorphism in Producer and AbstractProducer. (eventmesh)
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #3867:
URL: https://github.com/apache/eventmesh/pull/3867#issuecomment-2070569122
## [Codecov](https://app.codecov.io/gh/apache/eventmesh/pull/3867?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `38.46154%` with `16 lines` in your changes are missing coverage. Please review.
> Project coverage is 14.25%. Comparing base [(`0ce8d57`)](https://app.codecov.io/gh/apache/eventmesh/commit/0ce8d57e6a87890354fdc0643f06aae4acb6cc41?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`73ec3c0`)](https://app.codecov.io/gh/apache/eventmesh/pull/3867?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 431 commits behind head on master.
> :exclamation: Current head 73ec3c0 differs from pull request most recent head 2c5ca9e. Consider uploading reports for the commit 2c5ca9e to get more accurate results
| [Files](https://app.codecov.io/gh/apache/eventmesh/pull/3867?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...ntmesh/storage/rocketmq/producer/ProducerImpl.java](https://app.codecov.io/gh/apache/eventmesh/pull/3867?src=pr&el=tree&filepath=eventmesh-storage-plugin%2Feventmesh-storage-rocketmq%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Feventmesh%2Fstorage%2Frocketmq%2Fproducer%2FProducerImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZXZlbnRtZXNoLXN0b3JhZ2UtcGx1Z2luL2V2ZW50bWVzaC1zdG9yYWdlLXJvY2tldG1xL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9ldmVudG1lc2gvc3RvcmFnZS9yb2NrZXRtcS9wcm9kdWNlci9Qcm9kdWNlckltcGwuamF2YQ==) | 16.66% | [5 Missing :warning: ](https://app.codecov.io/gh/apache/eventmesh/pull/3867?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ge/rocketmq/producer/RocketMQAbstractProducer.java](https://app.codecov.io/gh/apache/eventmesh/pull/3867?src=pr&el=tree&filepath=eventmesh-storage-plugin%2Feventmesh-storage-rocketmq%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Feventmesh%2Fstorage%2Frocketmq%2Fproducer%2FRocketMQAbstractProducer.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZXZlbnRtZXNoLXN0b3JhZ2UtcGx1Z2luL2V2ZW50bWVzaC1zdG9yYWdlLXJvY2tldG1xL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9ldmVudG1lc2gvc3RvcmFnZS9yb2NrZXRtcS9wcm9kdWNlci9Sb2NrZXRNUUFic3RyYWN0UHJvZHVjZXIuamF2YQ==) | 63.63% | [3 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/eventmesh/pull/3867?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...entmesh/storage/knative/producer/ProducerImpl.java](https://app.codecov.io/gh/apache/eventmesh/pull/3867?src=pr&el=tree&filepath=eventmesh-storage-plugin%2Feventmesh-storage-knative%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Feventmesh%2Fstorage%2Fknative%2Fproducer%2FProducerImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZXZlbnRtZXNoLXN0b3JhZ2UtcGx1Z2luL2V2ZW50bWVzaC1zdG9yYWdlLWtuYXRpdmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9zdG9yYWdlL2tuYXRpdmUvcHJvZHVjZXIvUHJvZHVjZXJJbXBsLmphdmE=) | 25.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/eventmesh/pull/3867?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...eventmesh/storage/kafka/producer/ProducerImpl.java](https://app.codecov.io/gh/apache/eventmesh/pull/3867?src=pr&el=tree&filepath=eventmesh-storage-plugin%2Feventmesh-storage-kafka%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Feventmesh%2Fstorage%2Fkafka%2Fproducer%2FProducerImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZXZlbnRtZXNoLXN0b3JhZ2UtcGx1Z2luL2V2ZW50bWVzaC1zdG9yYWdlLWthZmthL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9ldmVudG1lc2gvc3RvcmFnZS9rYWZrYS9wcm9kdWNlci9Qcm9kdWNlckltcGwuamF2YQ==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/eventmesh/pull/3867?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ventmesh/storage/pulsar/producer/ProducerImpl.java](https://app.codecov.io/gh/apache/eventmesh/pull/3867?src=pr&el=tree&filepath=eventmesh-storage-plugin%2Feventmesh-storage-pulsar%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Feventmesh%2Fstorage%2Fpulsar%2Fproducer%2FProducerImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZXZlbnRtZXNoLXN0b3JhZ2UtcGx1Z2luL2V2ZW50bWVzaC1zdG9yYWdlLXB1bHNhci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZXZlbnRtZXNoL3N0b3JhZ2UvcHVsc2FyL3Byb2R1Y2VyL1Byb2R1Y2VySW1wbC5qYXZh) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/eventmesh/pull/3867?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #3867 +/- ##
============================================
+ Coverage 14.24% 14.25% +0.01%
+ Complexity 1320 1318 -2
============================================
Files 579 578 -1
Lines 28967 28927 -40
Branches 2802 2802
============================================
- Hits 4125 4123 -2
+ Misses 24450 24413 -37
+ Partials 392 391 -1
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/eventmesh/pull/3867?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org