#228 RFE: failure notifications for Maven builds
Opened 5 years ago by mikem. Modified 3 years ago

At present, maven builds do not send failure notifications. This is because the build entry is not created until after the buildMaven subtask is finished. When that subtask fails, there is no build info, so no entry and no notification. In fact, the parent task does not trap the exception, so it exits immediately.

Most likely, this will require some sort of new notification pathway. I doubt that the buildNotification task will serve.


I think we need always make sure the koji work flow could be done even the tasks failed early.

  1. the work flow codes should not raise any exception, or these code must catch all the exceptions, hence we can make sure the koji work flow not to be broken and to be ended by uncontrolled or unexpected conditions.
  2. since each step of the work flows need some well-crafted data, but a failed task perhaps cannot create such data. E.g., task handlers that send notifications need a build. Hence we need some fake data for this situations. Just like NULLL pointer in C lang. In one word, we need do a close-loop control model.

Short of completely refactoring how notifications work, we should probably just add new notifications for task completion and failure.

To avoid redundancy, we might want to have a way to suppress these in cases where there is already a build or tag notification.

the work flow codes should not raise any exception, or these code must catch all the exceptions, hence we can make sure the koji work flow not to be broken and to be ended by uncontrolled or unexpected conditions.

I think that is irrelevant here. Regardless of how the exceptions propagate here, the code reaches a point where the build cannot proceed, and yet there is no build info, so no way to form a coherent build notification.

Let's not go changing very deep assumptions in koji to solve this problem.

Ha, @mikem OK. will give a patch.

Does it make sense to create special taskFailedNotification? It will send notification for any failed task but it could be masked-out in children of build tasks.

I'm not in favour of adding taskNotification (spawned for every task start/end/fail) as it would generate too much traffic and tasks who no one really wants to see.

Now, even a task failed, the buildNotification will send a mail to the packagers. But because some code in the task handler suppose subtasks results always are dict, while this is not true when the subtasks are failed. This causes the buildNotification no chance to work. Hence I think there is no need to add a new task handler (if we add a new method to send notification).

Here are some questions about this issue:
1. xning's patch triggers the notification by BaseTaskHandler.wait(), so failures of all kinds of tasks will be noticed, even *NotificationTask themselves. Could we place it in HostExports.failTask() or Task.fail() to make it decoupling with TaskHandlers.
2. Could we finish this functionality as a postTaskStateChange plugin?
3. for MavenTask, buildNotification can not be triggered, because the exception of subtask "buildMaven" was not be caught in it's handler(), and at that moment build was not been created yet, so buildNotification doesn't fit this case. A simpler fix is triggering TaskNotification here
4. about how to make {build,tag}Notification and TaskNotification suppressed by each other, could we add a field like 'notification_enabled' in task table? To do it in code workflow may be too complex and hard to read.

Let's approach this as close the existing flow as we can. So

  • Let the maven task trap the error and trigger the notification
  • since there is build, we'll need a new task-based notification call for it to use
  • avoid any modifications to wait() or the db

It looks like we should catch errors in the maven task when it is waiting on the buildMaven tasks and trigger a task failure notification when an error happens there. This point is before there is a build to mark as failed.

Metadata Update from @julian8628:
- Issue set to the milestone: 1.15

4 years ago

Metadata Update from @mikem:
- Issue set to the milestone: 1.16 (was: 1.15)

4 years ago

Metadata Update from @tkopecek:
- Issue tagged with: feature

4 years ago

Metadata Update from @mikem:
- Issue set to the milestone: 1.17 (was: 1.16)

4 years ago

Metadata Update from @mikem:
- Issue set to the milestone: None (was: 1.17)
- Issue tagged with: backlog

3 years ago

Metadata Update from @dgregor:
- Custom field Size adjusted to None
- Issue priority set to: Low

3 years ago

Marking as low priority. The PR is close to done, but the approach isn't necessarily ideal. May want to revisit notifications as a whole.

Metadata Update from @dgregor:
- Issue tagged with: groomed

3 years ago

Login to comment on this ticket.

Metadata