[GitHub] [dolphinscheduler] ruanwenjun commented on a change in pull request #5715: [Fix-5714] When updating the existing alarm instance, the creation time should't be updated

[GitBox <gi...@apache.org>]

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: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org