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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
