You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by "ruanwenjun (via GitHub)" <gi...@apache.org> on 2025/12/23 12:46:29 UTC

Re: [PR] [Fix-17817][Master]Add logic to query projectUser in order to avoid NullPointerException when task or workflow timeout alerts are configured. [dolphinscheduler]

ruanwenjun commented on code in PR #17818:
URL: https://github.com/apache/dolphinscheduler/pull/17818#discussion_r2643071045


##########
dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/AlertDao.java:
##########
@@ -244,6 +244,13 @@ private void saveTaskTimeoutAlert(Alert alert, String content, int alertGroupId)
     public void sendTaskTimeoutAlert(WorkflowInstance workflowInstance,
                                      TaskInstance taskInstance,
                                      ProjectUser projectUser) {
+        assert projectUser != null;
+
+        // A null warningGroupId indicates that the user has explicitly configured a "no-alert" policy.
+        if (workflowInstance.getWarningGroupId() == null) {
+            return;
+        }

Review Comment:
   ```suggestion
   ```
   Don't add this kind of code, we needn't check at here, the method name is not `sendTaskTimeoutAlertIfNeeded`



##########
dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/AlertDao.java:
##########
@@ -244,6 +244,13 @@ private void saveTaskTimeoutAlert(Alert alert, String content, int alertGroupId)
     public void sendTaskTimeoutAlert(WorkflowInstance workflowInstance,
                                      TaskInstance taskInstance,
                                      ProjectUser projectUser) {
+        assert projectUser != null;

Review Comment:
   ```suggestion
           if(projectUser == null) {
                   throw IlleganArguementException("project user is null");
           }
   ```
   
   `assert` will do nothing if the jvm doesn't enable it.
   
   And I don't think we need to check the project user here, since this shouldn't affect the alert, alert only need to take care about alert plugin, if the project user is null, it only affects the alert content.



-- 
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