-
Notifications
You must be signed in to change notification settings - Fork 638
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 #4420] Add Feishu Sink connector #4522
Conversation
...r-feishu/src/main/java/org/apache/eventmesh/connector/feishu/server/FeishuConnectServer.java
Outdated
Show resolved
Hide resolved
...or-feishu/src/main/java/org/apache/eventmesh/connector/feishu/config/FeishuServerConfig.java
Outdated
Show resolved
Hide resolved
...r-feishu/src/test/java/org/apache/eventmesh/connector/s3/source/FeishuSinkConnectorTest.java
Show resolved
Hide resolved
@SunnyBoy-WYH ping, please take a look for the reveiw and fix the ci check error, thanks. |
ok,i will solve it later. thanks |
...or-feishu/src/main/java/org/apache/eventmesh/connector/feishu/config/FeishuServerConfig.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that you haven't fixed the previous CI error.
please approve the ci pipeline. |
# Conflicts: # eventmesh-common/src/main/java/org/apache/eventmesh/common/Constants.java
CI errors:
You can refer to https://eventmesh.apache.org/community/contribute/contribute#code-style to help you perform a checkstyle check locally. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #4522 +/- ##
============================================
+ Coverage 16.80% 16.87% +0.06%
- Complexity 1626 1638 +12
============================================
Files 749 753 +4
Lines 28683 28740 +57
Branches 2487 2489 +2
============================================
+ Hits 4821 4850 +29
- Misses 23408 23434 +26
- Partials 454 456 +2 ☔ View full report in Codecov by Sentry. |
Done |
import org.junit.jupiter.api.Test; | ||
|
||
@Disabled | ||
public class FeishuSinkConnectorTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you modify the code using Mockito stubbing or other mocking techniques to remove the annotation and make the test class runnable?
能否使用Mockito的打桩或者其他模拟的方式修改下代码,使该测试类可以移除掉该注解,并能实际运行?
} | ||
|
||
@Spy | ||
private FeishuSinkConnector feishuSinkConnector; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The @Spy
annotation is not very meaningful here, because you are not stubbing the object at all. You are basically constructing it manually.
这个@Spy
注解意义不大,因为您没有对该对象进行任何打桩,基本上是手动构建的。
|
||
@Test | ||
public void testFeishuSinkConnector() { | ||
assertDoesNotThrow(() -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that you are using Mockito for mocking, you can change the verification method of assertDoesNotThrow()
to verify the number of calls to the feishuClient
that you are mocking. Otherwise, the introduction of Mockito is meaningless.
现在您已经使用Mockito进行模拟了,就可以将assertDoesNotThrow()
的验证方式改成验证您所模拟的feishuClient
的调用次数。否则引入的Mockito就没有意义了。
final int times = 3; | ||
List<ConnectRecord> connectRecords = new ArrayList<>(); | ||
for (int i = 0; i < times; i++) { | ||
feishuSinkConnector.start(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One startup is enough. Additionally, it would be better to call stop ()
at the end of the test, although it is currently an empty method.
启动一次就够了。另外,测试结束时最好调用下stop()
,虽然目前是个空方法。
connectorConfig: | ||
connectorName: feishuSink | ||
reciveId: reciveIdValue | ||
reciveType: open_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reciveId
and recoverType
fields in the sink-config.yml
are different from the field names in SinkConnectorConfig
, which will cause the configuration to fail to be imported correctly.
@SunnyBoy-WYH 配置文件中reciveId和reciveType字段与SinkConnectorConfig中字段名不同,这将导致配置无法正确导入。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you willing to fix this bug? @hhuang1231 .
Additionally, this bug makes me worried about the correctness and effectiveness of this Connector in the absence of this bug.
您是否愿意修改这个bug?@hhuang1231 。另外,该bug也让我有点担心,当没有该bug时,该Connector的正确性和有效性。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pandaapo ok, I'll try to perfect it and make it work.
# Conflicts: # eventmesh-examples/src/main/resources/application.properties
Fixes #4420.
Motivation
[ISSUE #4420] Add Feishu Sink connector
Modifications
add sink connector for feishu:
Documentation