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

[ISSUE #367] Refactor connector #433

Merged
merged 3 commits into from
Jul 20, 2021

Conversation

ruanwenjun
Copy link
Member

1.support load plugin from eventMeshPluginDir
2.remove connector plugin from runtime

In local develop, we can add dependency of eventmesh-connector-rocketmq in build.gradle.
And remove the plugin when packing.

@codecov-commenter
Copy link

codecov-commenter commented Jul 13, 2021

Codecov Report

Merging #433 (a8a761e) into develop (db42296) will increase coverage by 0.04%.
The diff coverage is 42.27%.

❗ Current head a8a761e differs from pull request most recent head 5fc6c97. Consider uploading reports for the commit 5fc6c97 to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##             develop    #433      +/-   ##
============================================
+ Coverage       9.80%   9.84%   +0.04%     
- Complexity       282     283       +1     
============================================
  Files            228     228              
  Lines          10785   10829      +44     
  Branches         919     923       +4     
============================================
+ Hits            1057    1066       +9     
- Misses          9633    9666      +33     
- Partials          95      97       +2     
Impacted Files Coverage Δ
...h/connector/rocketmq/MessagingAccessPointImpl.java 38.46% <ø> (ø)
...pl/consumer/ConsumeMessageConcurrentlyService.java 0.00% <ø> (ø)
...eventmesh/connector/rocketmq/common/Constants.java 0.00% <ø> (ø)
.../connector/rocketmq/common/EventMeshConstants.java 0.00% <ø> (ø)
...ntmesh/connector/rocketmq/config/ClientConfig.java 44.92% <ø> (ø)
...connector/rocketmq/config/ClientConfiguration.java 0.00% <ø> (ø)
...onnector/rocketmq/config/ConfigurationWrapper.java 0.00% <ø> (ø)
.../connector/rocketmq/consumer/PushConsumerImpl.java 40.56% <ø> (ø)
...nector/rocketmq/consumer/RocketMQConsumerImpl.java 0.00% <ø> (ø)
...mesh/connector/rocketmq/domain/ConsumeRequest.java 0.00% <ø> (ø)
... and 20 more

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 db42296...5fc6c97. Read the comment docs.

@qqeasonchen
Copy link
Contributor

@ruanwenjun Please hellp to fix these new coming up conflicts.

@ruanwenjun ruanwenjun force-pushed the dev_wenjun_refactorPlugin branch 2 times, most recently from 7afa346 to 3606d55 Compare July 15, 2021 12:02
@ruanwenjun
Copy link
Member Author

@ruanwenjun Please hellp to fix these new coming up conflicts.

Done

@@ -16,7 +16,7 @@
*/

dependencies {
implementation project(":eventmesh-runtime"), project(":eventmesh-connector-rocketmq")
testImplementation project(":eventmesh-runtime"), project(":eventmesh-connector-rocketmq")
implementation project(":eventmesh-runtime")
Copy link
Contributor

Choose a reason for hiding this comment

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

The starter moudle is useless after refactor, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, if the starter module is only used to package dependencies, we can remove this module, but not sure if other things will need to be done in the future before runtime starting.

include 'eventmesh-sdk-java'
include 'eventmesh-common'
include 'eventmesh-connector-api'
include 'eventmesh-starter'
include 'eventmesh-test'
include 'eventmesh-spi'
include 'eventmesh-connector-plugin'
include 'eventmesh-connector-plugin:eventmesh-connector-rocketmq'

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to build all plugins without dependencies in gradle file and run specific plugin use configuration?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, we will not package the plugin into runtime, the all plugins will be packaged into jars, and copy to plugin directory.
We can configure the plugin directory.

@qqeasonchen
Copy link
Contributor

@ruanwenjun Did you test this already?

@ruanwenjun
Copy link
Member Author

ruanwenjun commented Jul 16, 2021

@ruanwenjun Did you test this already?

Yes, the documentation of SPI will be added latter.

@qqeasonchen
Copy link
Contributor

@ruanwenjun conflicts needs to be resolved.

@Jackzeng1224
Copy link

@ruanwenjun hello,
When I was using eventmesh-connector-plugin, I found that rocketMq could not be loaded. I called rocketMq according to the previous method. Is there any other places that need to be configured?
image

    1.support load plugin from eventMeshPluginDir
    2.remove connector plugin from runtime
@ruanwenjun
Copy link
Member Author

ruanwenjun commented Jul 19, 2021

@Jackzeng1224 Which branch do you use? Can you get the log

initialize extension instance success, extensionType: interface org.apache.eventmesh.api.producer.MeshMQProducer, extensionName: rocketmq

at StartUp log ? You can get some doc in this pr.

@ruanwenjun
Copy link
Member Author

@ruanwenjun conflicts needs to be resolved.

@qqeasonchen Thanks, done.

@Jackzeng1224
Copy link

initialize extension instance success,

@ruanwenjun hello, The branch I am in is https://github.com/ruanwenjun/incubator-eventmesh/tree/dev_wenjun_refactorPlugin. I haven't seen it so far “initialize extension instance success, extensionType: interface org.apache.eventmesh.api.producer.MeshMQProducer, extensionName: rocketmq”

image

@Jackzeng1224
Copy link

initialize extension instance success,

@ruanwenjun hello, The branch I am in is https://github.com/ruanwenjun/incubator-eventmesh/tree/dev_wenjun_refactorPlugin. I haven't seen it so far “initialize extension instance success, extensionType: interface org.apache.eventmesh.api.producer.MeshMQProducer, extensionName: rocketmq”

image

@ruanwenjun hello,I think this file configuration is not added.
image

@ruanwenjun
Copy link
Member Author

@Jackzeng1224 Yes, now the SPI module will load plugin from dist/plugin/connector(this can be changed by environment) and classpath. In local develop, you can execute dist task and copyConnectorPlugin to install the connector plugin.
Or you can add the rocketmq plugin module in build.gradle.

@ruanwenjun
Copy link
Member Author

@Jackzeng1224 In our design, we don't want to package all plugins in runtime jar.
The plugin is aim to separate the implementation and provide better scalability, user can choose to load the plugin by config.

@Jackzeng1224
Copy link

Jackzeng1224 commented Jul 20, 2021

@Jackzeng1224 Yes, now the SPI module will load plugin from dist/plugin/connector(this can be changed by environment) and classpath. In local develop, you can execute dist task and copyConnectorPlugin to install the connector plugin.
Or you can add the rocketmq plugin module in build.gradle.
OK!In the future, add the plug-in to eventmesh's build.gradle.

@ruanwenjun
Copy link
Member Author

@Jackzeng1224 How is it going, do you meet other problem?

@Jackzeng1224
Copy link

Jackzeng1224 commented Jul 20, 2021

@Jackzeng1224 How is it going, do you meet other problem?

No. Very well designed 👍

@qqeasonchen
Copy link
Contributor

When is the doc update?

@ruanwenjun Did you test this already?

Yes, the documentation of SPI will be added latter.

Have the docs updated now?

@ruanwenjun
Copy link
Member Author

@qqeasonchen Yes, the docs has been updated.

@qqeasonchen qqeasonchen merged commit 7850620 into apache:develop Jul 20, 2021
- eventmesh-connector-api : eventmesh插件接口定义模块
- eventmesh-connector-rocketmq : eventmesh rocketmq插件模块
- eventmesh-connector-api : eventmesh connector插件接口定义模块
- eventmesh-connector-plugin : eventmesh connector插件模块
Copy link
Contributor

Choose a reason for hiding this comment

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

eventmesh-connector-api was moved into eventmesh-connector-plugin,the description and picture are not updated

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I will submit a pr to fix.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I will submit a pr to fix.

See #323 , i want to let the student to fix it :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Great

@@ -44,4 +50,9 @@
<property name="format" value="^(org)\.apache(\.[a-zA-Z][a-zA-Z0-9]*)+$"/>
</module>
</module>

<module name="BeforeExecutionExclusionFileFilter">
<property name="fileNamePattern" value="./eventmesh-runtime/conf/sChat2.jks$"/>
Copy link
Member

Choose a reason for hiding this comment

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

Don't forget to remove this module :D

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok

jjz921024 pushed a commit to jjz921024/incubator-eventmesh that referenced this pull request Jul 25, 2021
* [ISSUE apache#367] Refactor connector
    1.support load plugin from eventMeshPluginDir
    2.remove connector plugin from runtime

* Add docs of plugin

* Remove connector-api in connector-plugin
xwm1992 pushed a commit to xwm1992/EventMesh that referenced this pull request Dec 27, 2021
* [ISSUE apache#367] Refactor connector
    1.support load plugin from eventMeshPluginDir
    2.remove connector plugin from runtime

* Add docs of plugin

* Remove connector-api in connector-plugin
xwm1992 pushed a commit that referenced this pull request Aug 4, 2022
* [ISSUE #367] Refactor connector
    1.support load plugin from eventMeshPluginDir
    2.remove connector plugin from runtime

* Add docs of plugin

* Remove connector-api in connector-plugin
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.

None yet

5 participants