Project

General

Profile

action #16276

notifications about failed and unreviewed jobs

Added by okurz over 3 years ago. Updated over 1 year ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Feature requests
Target version:
Start date:
2016-02-09
Due date:
% Done:

0%

Estimated time:
Difficulty:
Duration:

Description

user story

As a test reviewer I want to get informed about new failed and unreviewed jobs so that I can find

acceptance criteria

  • AC1: A failed job with no bugref or label is sent as an individual notification, e.g. using the AMQP IRC bridge in [#opensuse-factory](irc://chat.freenode.net/opensuse-factory) or [#openqa-test](irc://irc.suse.de/openqa-test)
  • AC2: A failed job with automatically carried over bugref triggers no such notification
  • AC3: The notification references the label content (, e.g. "bsc#1234" as link)

further notes

suggestion

provide logic that can find "failed unreviewed jobs", e.g. subscribe to job events, look for failed jobs, query openQA api for this job if there is any comment on the job, if not, publish a new event to be picked up by the amqp-irc bridge. This can be done within openQA or in a supporting infrastructure. Probably helpful as an individual event on AMQP.


Related issues

Copied from openQA Project - action #10672: notifications about new review comments availableResolved2016-02-09

Copied to openQA Project - action #17252: notifications to maintainer on failed modulesNew2016-02-09

History

#1 Updated by okurz over 3 years ago

  • Copied from action #10672: notifications about new review comments available added

#2 Updated by okurz over 3 years ago

  • Copied to action #17252: notifications to maintainer on failed modules added

#3 Updated by okurz about 3 years ago

  • Target version deleted (Milestone 6)

would be nice, didn't happen, just scrap the plan for now

#4 Updated by coolo over 2 years ago

  • Target version set to Ready

I guess a flag in job.done would do

#5 Updated by mkittler over 1 year ago

  • Status changed from New to In Progress
  • Assignee set to mkittler
  • Target version changed from Ready to Current Sprint

PR: https://github.com/os-autoinst/openQA/pull/1959

To compute the flag I need to find the bug reference anyways so I've just put the bug reference itself in the job.done event. If it is absent (set to null) one can consider the job unreviewed. The result (e.g. failed) is already part of the notification. So you basically have the information you need to figure that there's a failed and unreviewed job. Carried over bug references are considered as well (for AC2).

okurz Not sure what you mean with "as a link" in AC3 since this is just an AMQP message. But to ease creation of a link on the consumer-side I've also added the resolved bug URL (in case a bug reference is present).

#6 Updated by okurz over 1 year ago

Could work. https://github.com/okurz/openqa_irc/blob/master/Bot/BasicBot/Pluggable/Module/OpenQA.pm is an examplatory place where one can integrate the transformation into "human-digestable" format. Maybe you want to try that?

#7 Updated by mkittler over 1 year ago

I can try tomorrow.

#8 Updated by mkittler over 1 year ago

Some of the dependencies of that IRC bot are missing (not in devel:languages:perl and some not even in devel:languages:perl:CAPN:*). I tried to install missing Perl modules via cpan but that failed with a permission error trying to install something under /usr/bin. I don't want to mess my system for this so I didn't proceed.

Maybe it is better to add this to https://github.com/openSUSE/suse_msg anyways? That one I had successfully running on my system before and all its dependencies are packaged.

#9 Updated by mkittler over 1 year ago

So far we have:

[11:17] [Notice] -hermes an #openqa-events- openQA job finished with result failed: https://openqa.suse.de/t2390060

I could change that to:

[11:17] [Notice] -hermes an #openqa-events- openQA job finished with result failed (review outstanding): https://openqa.suse.de/t2390060

Then someone interested in those notifications could simply configure the IRC client to produce a notification for messages containing failed (review outstanding). That should be possible with Konversion. Not sure whether it is a common feature, though.

#10 Updated by okurz over 1 year ago

sure, sounds good. But maybe "pending" instead of "outstanding" due to the ambiguity? :)

#11 Updated by mkittler over 1 year ago

Yes, it is ambiguous so "pending" would be better, indeed. PR: https://github.com/openSUSE/suse_msg/pull/6

#12 Updated by dheidler over 1 year ago

@Marius: Tell me when the change in openQA is used in production so I can update the irc bot.

#13 Updated by mkittler over 1 year ago

It is already deployed on OSD.

#14 Updated by mkittler over 1 year ago

  • Status changed from In Progress to Resolved

Both PRs are merged.

#15 Updated by okurz over 1 year ago

  • Status changed from Resolved to In Progress

I am sorry but I would like to reopen as I assume that AC2 and AC3 are not yet covered. Could you please provide example IRC messages what we currently see? Do we need to clarify the requirements?

#16 Updated by mkittler over 1 year ago

Apparently it doesn't work in production. E.g. on #openqa-events we get [13:19] [Notice] -hermes an #openqa-events- openQA job finished with result failed (None): https://openqa.suse.de/t2425829. The brace should contain the bugref or 'review pending'. I have actually tested it locally (see PR).

dheidler Have you deployed the changes for the Python bot yet (and restarted the service)?

#17 Updated by mkittler over 1 year ago

Seems like the None is emitted when there's no bugref:

[12:11] [Notice] -hermes an #openqa-events- openQA job finished with result failed (None): https://openqa.suse.de/t2430595
[12:11] [Notice] -hermes an #openqa-events- openQA job restarted: https://openqa.suse.de/t2430831
[12:11] [Notice] -hermes an #openqa-events- openQA job finished with result passed: https://openqa.suse.de/t2430997
[12:12] [Notice] -hermes an #openqa-events- openQA job finished with result failed (bsc#1073390): https://openqa.suse.de/t2429267

So likely just a mistake on the Python-side where my change is obviously already deployed.

#18 Updated by mkittler over 1 year ago

  • Status changed from In Progress to Resolved

Works now after fixing a slight mistake on the Python-side, e.g.:

[16:08] [Notice] -hermes an #openqa-events- openQA job finished with result failed (bsc#1106085): https://openqa.suse.de/t2439840
[16:10] [Notice] -hermes an #openqa-events- openQA job finished with result failed (review pending): https://openqa.suse.de/t2440097

#19 Updated by okurz over 1 year ago

I appreciate the work you did but I doubt we are there to have the user story fulfilled. If you prefer we can continue in another ticket but have to be senseful about what users of openQA ask for: Same as in #16276#note-15, Are you sure AC2 is covered? How about we talk in person next week in office when you address me?

#20 Updated by okurz over 1 year ago

  • Status changed from Resolved to Feedback

#21 Updated by mkittler over 1 year ago

Right, AC2 is not covered. So the case of carry-over must be distinguishable.

#22 Updated by mkittler over 1 year ago

I have just added a flag to tell whether the bug was carried-over. But when writing the test for it I've noticed that this is actually not that useful. I would have to construct the rare (test) case where a bugref is present at the time the job fails but it hasn't been added via take over but instead by a user while the test was still running.

And thinking further, you actually don't even need to distinguish the carry over. If there's no bugref at the time the job failed you will get "review pending" on IRC. That implies that no bugref has been carried over. Otherwise you wouldn't get "review pending" and the carried over bugref instead. So just setup your IRC client to notify you if a message contains "review pending" and you're good. So AC2 is fulfilled, indeed.

Since I have implemented it anyways, we can still add an explicit "carried over flag" to the AMQP message if you like. Just the test for the negative case would be missing.

#23 Updated by mkittler over 1 year ago

  • Status changed from Feedback to Resolved

Also available in: Atom PDF