You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@eventmesh.apache.org by GitBox <gi...@apache.org> on 2021/09/21 18:57:27 UTC
[GitHub] [incubator-eventmesh] yzhao244 opened a new pull request #531: create topic command
yzhao244 opened a new pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531
<!--
### Contribution Checklist
- Name the pull request in the form "[ISSUE #XXXX] Title of the pull request",
where *XXXX* should be replaced by the actual issue number.
Skip *[ISSUE #XXXX]* if there is no associated github issue for this pull request.
- Fill out the template below to describe the changes contributed by the pull request.
That will give reviewers the context they need to do the review.
- Each pull request should address only one issue.
Please do not mix up code from multiple issues.
- Each commit in the pull request should have a meaningful commit message.
- Once all items of the checklist are addressed, remove the above text and this checklist,
leaving only the filled out template below.
(The sections below can be removed for hotfixes of typos)
-->
<!--
(If this PR fixes a GitHub issue, please add `Fixes ISSUE#<XXX>`.)
-->
Fixes ISSUE #346
### Motivation
Add enhancement to eventmesh-connector-plugin module to support of creating new topic in rocketmq. This is part of the lifecycle management of rocketmq topics.
### Modifications
New API implementation has done to eventmesh-connector-api and eventmesh-connector-rocketmq to support of creating rocketmq topics.
### Documentation
- Does this pull request introduce a new feature? (yes / no)
- If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
- If a feature is not applicable for documentation, explain why?
- If a feature is not documented yet in this PR, please create a followup issue for adding the documentation
--
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
[GitHub] [incubator-eventmesh] yzhao244 commented on a change in pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
yzhao244 commented on a change in pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#discussion_r758026439
##########
File path: eventmesh-connector-plugin/eventmesh-connector-rocketmq/src/test/java/org/apache/rocketmq/producer/RocketMQProducerImplTest.java
##########
@@ -0,0 +1,60 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.rocketmq.producer;
+
+
+import org.apache.eventmesh.api.producer.MeshMQProducer;
+import org.apache.eventmesh.connector.rocketmq.producer.RocketMQProducerImpl;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import io.openmessaging.api.exception.OMSRuntimeException;
+
+public class RocketMQProducerImplTest {
+
+ @Before
+ public void before() {}
+
+
+ @After
+ public void after() {
+ //TBD:Remove topic
+ }
+
+ @Test
+ public void testCreate_Ok() {
+
+ MeshMQProducer meshMQProducer = new RocketMQProducerImpl();
+ try {
+ meshMQProducer.createTopic("");
+ Assert.assertTrue("Failed to detect empty topic", false);
+ } catch (OMSRuntimeException e) {
+ Assert.assertTrue("Successfully detected empty topic", true);
Review comment:
fixed.
> @qqeasonchen meshMQProducer and other names are strange. we could remove the concrete storage name in the code. maybe meshPub is a good name.
@vongosling changed name to meshPub, thx for the suggestion of the name. :)
--
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
[GitHub] [incubator-eventmesh] qqeasonchen commented on pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
qqeasonchen commented on pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#issuecomment-945407116
@yzhao244 please help to fix the conflicts.
--
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
[GitHub] [incubator-eventmesh] yzhao244 commented on pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
yzhao244 commented on pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#issuecomment-925029061
> please supply the unit test for this feature, thanks
thx, I will address this.
--
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
[GitHub] [incubator-eventmesh] yzhao244 commented on pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
yzhao244 commented on pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#issuecomment-981276317
@qqeasonchen @vongosling finally, all fixed lol.
--
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
[GitHub] [incubator-eventmesh] yzhao244 commented on pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
yzhao244 commented on pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#issuecomment-925091173
> > It seems in your design, We need to implement admin module for every connector, can we just add a new method in `MeshMQProducer`, so the `MQAdmin` can just rely on the connector to create topic, this may be helpful if we add audit in the future.
>
> Good point. Let me rework my implementation.
Addressed, please review
--
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
[GitHub] [incubator-eventmesh] yzhao244 edited a comment on pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
yzhao244 edited a comment on pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#issuecomment-925155122
> It seems in your design, We need to implement admin module for every connector, can we just add a new method in `MeshMQProducer`, so the `MQAdmin` can just rely on the connector to create topic, this may be helpful if we add audit in the future.
After adding the new createTopic method to "MeshMQProducer", both StandaloneMeshMQProducerAdaptor.java and RocketMQProducerImpl.java require this new method implemented.
--
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
[GitHub] [incubator-eventmesh] yzhao244 commented on pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
yzhao244 commented on pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#issuecomment-980511442
@qqeasonchen fixed most of the code style check issues.. The remaining ones are a bit tricky. e.x "Abbreviation in name 'RocketMQProducerImplTest' must contain no more than '1' consecutive capital letters. " . It is about renaming "RocketMQ" to be "RocketMq" which doesn't look right in eventmesh project. :) .. Please let me know if I miss anything
--
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
[GitHub] [incubator-eventmesh] yzhao244 commented on pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
yzhao244 commented on pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#issuecomment-925155122
> It seems in your design, We need to implement admin module for every connector, can we just add a new method in `MeshMQProducer`, so the `MQAdmin` can just rely on the connector to create topic, this may be helpful if we add audit in the future.
After adding the new createTopic method, both StandaloneMeshMQProducerAdaptor.java and RocketMQProducerImpl.java require this new method implemented.
--
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
[GitHub] [incubator-eventmesh] yzhao244 commented on a change in pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
yzhao244 commented on a change in pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#discussion_r758486271
##########
File path: eventmesh-connector-plugin/eventmesh-connector-rocketmq/src/main/resources/rocketmq-client.properties
##########
@@ -16,3 +16,6 @@
#
#######################rocketmq-client##################
eventMesh.server.rocketmq.namesrvAddr=127.0.0.1:9876;127.0.0.1:9876
+eventMesh.server.rocketmq.cluster=********
Review comment:
thx, fixed
--
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
[GitHub] [incubator-eventmesh] xwm1992 merged pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
xwm1992 merged pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531
--
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
[GitHub] [incubator-eventmesh] yzhao244 edited a comment on pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
yzhao244 edited a comment on pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#issuecomment-980511442
@qqeasonchen @vongosling fixed most of the code style check issues.. The remaining ones are a bit tricky. e.x "Abbreviation in name 'RocketMQProducerImplTest' must contain no more than '1' consecutive capital letters. " . It is about renaming "RocketMQ" to be "RocketMq" which doesn't look right in eventmesh project. :) .. Please let me know if I miss anything
--
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
[GitHub] [incubator-eventmesh] vongosling commented on pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
vongosling commented on pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#issuecomment-980485414
@yzhao244 many checkstyle checks failure, would you like to resolve it?
--
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
[GitHub] [incubator-eventmesh] qqeasonchen commented on pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
qqeasonchen commented on pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#issuecomment-981356802
> @qqeasonchen @vongosling @xwm1992 finally, all fixed lol.
nice job!
--
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
[GitHub] [incubator-eventmesh] ruanwenjun commented on a change in pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on a change in pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#discussion_r758317598
##########
File path: eventmesh-connector-plugin/eventmesh-connector-rocketmq/src/main/resources/rocketmq-client.properties
##########
@@ -16,3 +16,6 @@
#
#######################rocketmq-client##################
eventMesh.server.rocketmq.namesrvAddr=127.0.0.1:9876;127.0.0.1:9876
+eventMesh.server.rocketmq.cluster=********
Review comment:
Can we add a default value here? Maybe it can run success in most times.
##########
File path: eventmesh-connector-plugin/eventmesh-connector-rocketmq/src/test/java/org/apache/eventmesh/producer/DefaultProducerImplTest.java
##########
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.eventmesh.producer;
+
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.apache.eventmesh.api.producer.MeshMQProducer;
+import org.apache.eventmesh.connector.rocketmq.producer.RocketMQProducerImpl;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import io.openmessaging.api.exception.OMSRuntimeException;
+
+public class DefaultProducerImplTest {
+
+ @Before
+ public void before() {}
+
+
+ @After
+ public void after() {
+ //TBD:Remove topic
+ }
+
+ @Test
+ public void testCreate_EmptyTopic() {
+ MeshMQProducer meshPub = new RocketMQProducerImpl();
+ try {
+ meshPub.createTopic("");
+ } catch (OMSRuntimeException e) {
+ assertThat(e.getMessage()).isEqualTo("RocketMQ can not create topic .");
Review comment:
```suggestion
assertThat(e.getMessage()).isEqualTo("RocketMQ can not create topic.");
```
##########
File path: eventmesh-connector-plugin/eventmesh-connector-rocketmq/src/main/java/org/apache/eventmesh/connector/rocketmq/producer/RocketMQProducerImpl.java
##########
@@ -149,4 +158,16 @@ public void updateCredential(Properties credentialProperties) {
public <T> MessageBuilder<T> messageBuilder() {
return null;
}
+
+ @Override
+ public void createTopic(String topicName) throws OMSRuntimeException {
+ CreateTopicCommand createTopicCommand = new CreateTopicCommand();
+ createTopicCommand.setTopicName(topicName);
+ try {
+ createTopicCommand.execute();
+ } catch (Exception e) {
+ throw new OMSRuntimeException(-1,
+ String.format("RocketMQ can not create topic %s.", topicName), e);
Review comment:
It might be better don't add an error code casually here.
##########
File path: eventmesh-connector-plugin/eventmesh-connector-rocketmq/src/main/java/org/apache/eventmesh/connector/rocketmq/producer/RocketMQProducerImpl.java
##########
@@ -149,4 +158,16 @@ public void updateCredential(Properties credentialProperties) {
public <T> MessageBuilder<T> messageBuilder() {
return null;
}
+
+ @Override
+ public void createTopic(String topicName) throws OMSRuntimeException {
+ CreateTopicCommand createTopicCommand = new CreateTopicCommand();
+ createTopicCommand.setTopicName(topicName);
+ try {
+ createTopicCommand.execute();
+ } catch (Exception e) {
+ throw new OMSRuntimeException(-1,
+ String.format("RocketMQ can not create topic %s.", topicName), e);
Review comment:
```suggestion
throw new OMSRuntimeException(
String.format("RocketMQ can not create topic %s.", topicName), e);
```
--
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
[GitHub] [incubator-eventmesh] xwm1992 commented on pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
xwm1992 commented on pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#issuecomment-924555210
please supply the unit test for this feature, thanks
--
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
[GitHub] [incubator-eventmesh] yzhao244 commented on pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
yzhao244 commented on pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#issuecomment-980485047
@qqeasonchen conflict fixed. is it urgent to fix the code style check error? :)
--
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
[GitHub] [incubator-eventmesh] vongosling commented on a change in pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
vongosling commented on a change in pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#discussion_r757980844
##########
File path: eventmesh-connector-plugin/eventmesh-connector-rocketmq/src/test/java/org/apache/rocketmq/producer/RocketMQProducerImplTest.java
##########
@@ -0,0 +1,60 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.rocketmq.producer;
+
+
+import org.apache.eventmesh.api.producer.MeshMQProducer;
+import org.apache.eventmesh.connector.rocketmq.producer.RocketMQProducerImpl;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import io.openmessaging.api.exception.OMSRuntimeException;
+
+public class RocketMQProducerImplTest {
+
+ @Before
+ public void before() {}
+
+
+ @After
+ public void after() {
+ //TBD:Remove topic
+ }
+
+ @Test
+ public void testCreate_Ok() {
+
+ MeshMQProducer meshMQProducer = new RocketMQProducerImpl();
+ try {
+ meshMQProducer.createTopic("");
+ Assert.assertTrue("Failed to detect empty topic", false);
+ } catch (OMSRuntimeException e) {
+ Assert.assertTrue("Successfully detected empty topic", true);
Review comment:
Do not assert exception; One method should handle one exception.
--
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
[GitHub] [incubator-eventmesh] qqeasonchen commented on pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
qqeasonchen commented on pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#issuecomment-981235597
> @qqeasonchen meshMQProducer and other names are strange. we could remove the concrete storage name in the code. maybe meshPub is a good name.
agree, @xwm1992 please follow it.
--
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
[GitHub] [incubator-eventmesh] yzhao244 edited a comment on pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
yzhao244 edited a comment on pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#issuecomment-981276317
@qqeasonchen @vongosling @xwm1992 finally, all fixed lol.
--
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
[GitHub] [incubator-eventmesh] yzhao244 commented on pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
yzhao244 commented on pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#issuecomment-926618073
> > > It seems in your design, We need to implement admin module for every connector, can we just add a new method in `MeshMQProducer`, so the `MQAdmin` can just rely on the connector to create topic, this may be helpful if we add audit in the future.
> >
> >
> > After adding the new createTopic method to "MeshMQProducer", both StandaloneMeshMQProducerAdaptor.java and RocketMQProducerImpl.java require this new method implemented.
>
> It's ok, standalone mode can supply the empty implementation for create topic , it's just a example for quickstart.
Cool, I have made code changes to address the review comments, please review. :)
--
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
[GitHub] [incubator-eventmesh] yzhao244 edited a comment on pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
yzhao244 edited a comment on pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#issuecomment-926618073
> > > It seems in your design, We need to implement admin module for every connector, can we just add a new method in `MeshMQProducer`, so the `MQAdmin` can just rely on the connector to create topic, this may be helpful if we add audit in the future.
> >
> >
> > After adding the new createTopic method to "MeshMQProducer", both StandaloneMeshMQProducerAdaptor.java and RocketMQProducerImpl.java require this new method implemented.
>
> It's ok, standalone mode can supply the empty implementation for create topic , it's just a example for quickstart.
@ruanwenjun Cool, I have made code changes to address the review comments, please review. :)
--
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
[GitHub] [incubator-eventmesh] vongosling commented on pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
vongosling commented on pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#issuecomment-981199524
@qqeasonchen meshMQProducer and other names are strange. we could remove the concrete storage name in the code. maybe meshPub is a good name.
--
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
[GitHub] [incubator-eventmesh] ruanwenjun commented on a change in pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on a change in pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#discussion_r758317205
##########
File path: eventmesh-connector-plugin/eventmesh-connector-rocketmq/src/main/java/org/apache/eventmesh/connector/rocketmq/producer/RocketMQProducerImpl.java
##########
@@ -149,4 +158,16 @@ public void updateCredential(Properties credentialProperties) {
public <T> MessageBuilder<T> messageBuilder() {
return null;
}
+
+ @Override
+ public void createTopic(String topicName) throws OMSRuntimeException {
+ CreateTopicCommand createTopicCommand = new CreateTopicCommand();
+ createTopicCommand.setTopicName(topicName);
+ try {
+ createTopicCommand.execute();
+ } catch (Exception e) {
+ throw new OMSRuntimeException(-1,
+ String.format("RocketMQ can not create topic %s.", topicName), e);
Review comment:
It might be better don't add an error code casually here, if you hope to add an error code, you should define it well.
--
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
[GitHub] [incubator-eventmesh] xwm1992 commented on pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
xwm1992 commented on pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#issuecomment-982362871
> > @qqeasonchen meshMQProducer and other names are strange. we could remove the concrete storage name in the code. maybe meshPub is a good name.
>
> agree, @xwm1992 please follow it.
ok, I'll change `meshMQProducer` to `meshPub`.
--
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
[GitHub] [incubator-eventmesh] yzhao244 commented on pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
yzhao244 commented on pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#issuecomment-925091383
> > please supply the unit test for this feature, thanks
>
> thx, I will address this.
Addressed, please review
--
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
[GitHub] [incubator-eventmesh] ruanwenjun commented on a change in pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on a change in pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#discussion_r713589894
##########
File path: eventmesh-connector-plugin/eventmesh-connector-api/src/main/java/org/apache/eventmesh/api/admin/MeshMQAdmin.java
##########
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.eventmesh.api.admin;
+
+import org.apache.eventmesh.spi.EventMeshSPI;
+
+@EventMeshSPI(isSingleton = false)
Review comment:
It seems this plugin instance can be a singleton.
```suggestion
@EventMeshSPI(isSingleton = true)
```
--
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
[GitHub] [incubator-eventmesh] qqeasonchen commented on pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
qqeasonchen commented on pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#issuecomment-979660458
@yzhao244 could you fix the conflicts and let it can be merged.
--
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
[GitHub] [incubator-eventmesh] yzhao244 commented on pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
yzhao244 commented on pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#issuecomment-925029692
> It seems in your design, We need to implement admin module for every connector, can we just add a new method in `MeshMQProducer`, so the `MQAdmin` can just rely on the connector to create topic, this may be helpful if we add audit in the future.
Good point. Let me rework my 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: 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
[GitHub] [incubator-eventmesh] xwm1992 commented on pull request #531: create topic command
Posted by GitBox <gi...@apache.org>.
xwm1992 commented on pull request #531:
URL: https://github.com/apache/incubator-eventmesh/pull/531#issuecomment-926402975
> > It seems in your design, We need to implement admin module for every connector, can we just add a new method in `MeshMQProducer`, so the `MQAdmin` can just rely on the connector to create topic, this may be helpful if we add audit in the future.
>
> After adding the new createTopic method to "MeshMQProducer", both StandaloneMeshMQProducerAdaptor.java and RocketMQProducerImpl.java require this new method implemented.
It's ok, standalone mode can supply the empty implementation for create topic , it's just a example for quickstart.
--
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