ruanwenjun commented on a change in pull request #5715:
URL: https://github.com/apache/dolphinscheduler/pull/5715#discussion_r661151501
##########
File path:
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/AlertPluginInstanceServiceImpl.java
##########
@@ -109,12 +110,19 @@
@Override
public Map<String, Object> update(User loginUser, int pluginInstanceId,
String instanceName, String pluginInstanceParams) {
- AlertPluginInstance alertPluginInstance = new AlertPluginInstance();
+ Map<String, Object> result = new HashMap<>();
+
+ AlertPluginInstance alertPluginInstance =
alertPluginInstanceMapper.queryById(pluginInstanceId);
+
+ if (alertPluginInstance == null) {
+ putMsg(result, Status.QUERY_PLUGINS_RESULT_IS_NULL,
pluginInstanceId);
Review comment:
You need to check the msg of `Status.QUERY_PLUGINS_RESULT_IS_NULL`,
right now the msg doesn't include a placeholder. And it might be better to use
instanceName instead of pluginInstanceId in the output msg.
```
QUERY_PLUGINS_RESULT_IS_NULL(110002, "query plugins result is null",
"查询插件为空"),
```
In addition to that, I am not sure if we need to query the pluginInstance
here in line 115. If you just want to fix the createTime, you can add a new
constructor in `AlertPluginInstance` instead of using the below constructor.
```java
public AlertPluginInstance() {
this.createTime = new Date();
this.updateTime = new Date();
}
```
If you want to fix the problem that the plugin doesn't exist, the line 132
seems already done.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
