Project

General

Profile

action #98637

coordination #97121: [epic] enable qa-maintenance/openQABot comments on smelt again

[timeboxed:20h] try to enable comments on IBS (and smelt) again from SUSE QA maintenance openQA test results size:M

Added by okurz 3 months ago. Updated 12 days ago.

Status:
Resolved
Priority:
High
Assignee:
Target version:
Start date:
2021-09-14
Due date:
% Done:

80%

Estimated time:

Description

Motivation

See parent #97121

Acceptance criteria

  • AC1: release requests on IBS show again comments whenever related openQA incident tests fail

Suggestions

  • We can't re-enable the feature in the old bot because of unrelated changes
  • Likely the best approach is to port the "IBS commenting" feature from qa-maintenance/openQABot to qa-maintenance/bot-ng.
  • Also see #97121#note-17 for multiple alternative approaches we can take.
  • Re-run the code locally to see if it works (there's no test suite)

Out of scope

  • Re-consider what type of comments or where we want them - tickets can be filed for different features

History

#1 Updated by okurz 3 months ago

  • Description updated (diff)

#2 Updated by cdywan 3 months ago

  • Subject changed from enable comments on IBS (and smelt) again from SUSE QA maintenance openQA test results to [timeboxed:20h] try to enable comments on IBS (and smelt) again from SUSE QA maintenance openQA test results size:M
  • Description updated (diff)
  • Status changed from New to Workable
  • Priority changed from Urgent to High

Reducing priority because we don't have the knowledge to handle this urgently.

#3 Updated by cdywan 3 months ago

  • Status changed from Workable to In Progress
  • Assignee set to cdywan

I forked and checked out the repos to start with, let's see what I'll come up with once I have some basic setup and find out how the relevant code paths work.

#4 Updated by okurz 3 months ago

Stephan Barth representing maintsec in openqa-review stated Quick feedback: our team is quite happy to hear that you want to bring them back 🙂️ so one more reason to have that feature back.

#5 Updated by openqa_review 3 months ago

  • Due date set to 2021-10-01

Setting due date based on mean cycle time of SUSE QE Tools

#6 Updated by osukup 2 months ago

Likely the best approach is to port the "IBS commenting" feature from qa-maintenance/openQABot to qa-maintenance/bot-ng.

not this isn't best approach .. one of the motivations to move from openQABot was spam generated by commenting in IBS ..., so you still want spam all people with comments.

Best approach is to have SMELT read needed info from dahsboard.qam.suse.de API and then show this info in SMELT incident/ RR ..

#7 Updated by cdywan 2 months ago

osukup wrote:

Likely the best approach is to port the "IBS commenting" feature from qa-maintenance/openQABot to qa-maintenance/bot-ng.

not this isn't best approach .. one of the motivations to move from openQABot was spam generated by commenting in IBS ..., so you still want spam all people with comments.

Best approach is to have SMELT read needed info from dahsboard.qam.suse.de API and then show this info in SMELT incident/ RR ..

See out of scope. Proposal to involve the dashboard weren't discussed properly. This is a High ticket, at best it should be the minimum effort.

#8 Updated by okurz 2 months ago

osukup wrote:

not this isn't best approach .. one of the motivations to move from openQABot was spam generated by commenting in IBS ..., so you still want spam all people with comments.

It's only "spam" if it's messages that people never subscribed to. IIUC only reviewees, reviewers and subscribers to qa-maintenance-reports@suse.de receive these emails. If people need to be on the reviewer list but don't want to receive these messages the best approach I can suggest right now is for individuals to configure according email filters. The motivation should be clear from this ticket and the parent #97121 . For now smelt does not have any potential future feature that one envisions, removing comments broke existing workflows and maintsec confirmed that bringing back the comments is important and would be appreciated.

#9 Updated by cdywan 2 months ago

  • Status changed from In Progress to Workable
  • Assignee deleted (cdywan)

Un-assigning for now, since I didn't have the capacity to get well-enough acquainted with the code on my own and had to investigate more pressing infra issues.

#10 Updated by okurz 2 months ago

  • Due date deleted (2021-10-01)

#11 Updated by cdywan about 1 month ago

Maybe we can document some manual testing steps e.g. used here https://progress.opensuse.org/issues/97955#note-6 to help with this ticket

#12 Updated by cdywan about 1 month ago

Discussed this with Tina briefly. The idea behind this ticket was not to try and add test coverage first - since we lack basic coverage - but not having tests seem to prevent the ticket from being picked up.

#13 Updated by okurz about 1 month ago

cdywan wrote:

Discussed this with Tina briefly. The idea behind this ticket was not to try and add test coverage first - since we lack basic coverage - but not having tests seem to prevent the ticket from being picked up.

Sounds like you are running in circles :D Again please just try to get this over with, hard-core hacky solution is fine. This does not need to be anything long-term sustainable at this point in time.

#14 Updated by jbaier_cz about 1 month ago

  • Status changed from Workable to In Progress
  • Assignee set to jbaier_cz

#15 Updated by openqa_review about 1 month ago

  • Due date set to 2021-11-16

Setting due date based on mean cycle time of SUSE QE Tools

#16 Updated by jbaier_cz about 1 month ago

  • % Done changed from 0 to 50

I was able to resurrect the commenting code and transform the message making procedure to use the new data format. The last missing piece is the commenting itself as it was also tied to incident approval with is now in separate and done differently.

#17 Updated by jbaier_cz 27 days ago

  • % Done changed from 50 to 80

I added the comment reading/finding/writing code. To be production ready, it still missing some convenient features and computing additional info for the comments (like openqa revision in the original version).

#18 Updated by jbaier_cz 24 days ago

  • Status changed from In Progress to Feedback

#19 Updated by cdywan 18 days ago

  • Status changed from Feedback to Resolved

There's one example mentioned https://build.opensuse.org/request/show/925479#comment-1546655 so let's assume we're good.

#20 Updated by okurz 17 days ago

  • Status changed from Resolved to Feedback

Is the PR even merged?

#21 Updated by cdywan 17 days ago

  • Status changed from Feedback to Resolved

okurz wrote:

Is the PR even merged?

Yes. I did not create the bot comment manually :-D

#22 Updated by okurz 17 days ago

  • Status changed from Resolved to Feedback

Still please let's see this working in production. Same as coolo commented in https://suse.slack.com/archives/C02AJ1E568M/p1637320998049400 "the given comment is at build.opensuse.org - and a completely different bot is creating them".

#23 Updated by cdywan 17 days ago

okurz wrote:

Still please let's see this working in production. Same as coolo commented in https://suse.slack.com/archives/C02AJ1E568M/p1637320998049400 "the given comment is at build.opensuse.org - and a completely different bot is creating them".

You don't need to state the obvious. Apparently oqamaint triggered that comment, and mention in the MR itself was wrong.

osukup jbaier_cz So how do I verify the fix?

#24 Updated by jbaier_cz 14 days ago

  • Due date changed from 2021-11-16 to 2021-12-03

I assume you mean how to test the new feature (it is not a fix, it is a new functionality). As also noted in the readme, one need to invoke the new command inc-comment with the production token for osc or we can extend this ticket and enhance the (soon to be existing) test suite. As the MR was not merged before my vacation, I was not able to run it in the CI, so I do not think we can find example comments in the IBS (unless someone else did the run manually).

Bumping the due date as I cannot effectively work on this one this week (vacation). Someone can take over if this ticket cannot wait to the next week.

#25 Updated by mkittler 14 days ago

SR for invoking the new command inc-comment within bot-ng's GitLab pipeline: https://gitlab.suse.de/qa-maintenance/bot-ng/-/merge_requests/36

#26 Updated by cdywan 14 days ago

mkittler wrote:

SR for invoking the new command inc-comment within bot-ng's GitLab pipeline: https://gitlab.suse.de/qa-maintenance/bot-ng/-/merge_requests/36

#27 Updated by mkittler 14 days ago

It looks like we cannot run inc-comment in its current form because it would lead to spamming too much mails (see https://gitlab.suse.de/qa-maintenance/bot-ng/-/merge_requests/36#note_363914).

#28 Updated by okurz 12 days ago

  • Due date deleted (2021-12-03)
  • Status changed from Feedback to Resolved

Seems like this timeboxed task was a success because we do have the possibility to send comments, similar to as before. We have a merge request merged that allows to call the command and also a MR prepared by mkittler that shows how we could enable automatic commenting from gitlab CI pipelines. This has not been merged and put into production due to the referenced discussion and objection by coolo. This can be solved as part of the parent epic, the task here is successfully completed, hence resolving.

Also available in: Atom PDF