#94 BlockerBugs async review discussion PoC
Closed: Fixed a year ago by kparal. Opened 2 years ago by kparal.

Per https://pagure.io/fedora-qa/issue/601 we want to integrate BBA with Pagure to allow review discussions outside of meetings. This is a ticket to track action items and problems found during the initial PoC implementation.

PoC BBA: http://blockerbugs-blockerbugs.apps.os.fedorainfracloud.org/
PoC discussions: https://stg.pagure.io/fedora-qa/blocker-review

Problems and tasks:
- [✓] https://partner-bugzilla.redhat.com/ doesn't include F32 blocker trackers from https://bugzilla.redhat.com/ . I solved it by creating those bugs by hand and special-casing those bug numbers in init_db.sh.
- [✓] Ticket discussions are created even for Prioritized Bugs. We need to avoid doing that. Only blockers and FEs get a discussion ticket.
- [✓] Make the initial ticket title & description a configurable template (probably in markdown format with python templating strings) as a standalone file, not hardcoded in code
- [✓] Implement a vote watcher that updates vote summary (depends on https://pagure.io/pagure/issue/4704 ). An example summary is here.
- [✓] Deploy new BBA in Docker on RHEL or on Fedora, because we now need Python 3.6+. This is being worked on by @frantisekz. Done: http://blockerbugs-blockerbugs.apps.os.fedorainfracloud.org/
- [✓] Create a QA bot account (or figure out if we have one already) that will be used to generate Pagure API token keys and submit comments if needed. An infra request: https://pagure.io/fedora-infrastructure/issue/8566 . Done: we now have a blockerbot FAS/Pagure account.
- [✓] Create a blocker review project on Pagure. Done: https://pagure.io/fedora-qa/blocker-review and https://stg.pagure.io/fedoraqa-test https://stg.pagure.io/fedora-qa/blocker-review
- [✓] Once we have a project on production Pagure and want to deploy new BBA, create an API token and ask infra to extend its lifetime from default 60 days to e.g. 1 year. See https://pagure.io/fedora-infrastructure/issue/8520 . Update: ticket here: https://pagure.io/fedora-infrastructure/issue/8573
- [✓] Extend blockerbugs cli to be able to re-create a discussion ticket even if it already exists (when something goes wrong).
- [✓] Add a config option that allows to enable/disable the discussion bot (when something goes really wrong).


[✓] Add access control so that admin commands can only be performed by members of a particular Pagure group. @lbrabec has some initial code here:
https://pagure.io/fedora-qa/blockerbugs/commits/feature/acceptmembersonly?identifier=feature%2Facceptmembersonly
We need to ensure we have this covered by tests and that we don't actually touch Pagure during running the test suite (mock the access). Also, the list should be retrieved at most once during processing (we should retrieve it every time we process some comment - even old admin comments).

Metadata Update from @kparal:
- Issue status updated to: Open (was: Closed)

2 years ago

[✓] Fix code to not require justification after AGREED. This should work and didn't:

AGREED AcceptedBetaBlocker
AGREED ACCEPTEDFINALFE

https://stg.pagure.io/fedora-qa/blocker-review/issue/11#comment-260376
Please cover with tests.

[✓] Make sure the blockerbot doesn't go into a loop when a vote is agreed on, yet nobody voted on it yet:
https://stg.pagure.io/fedora-qa/blocker-review/issue/11#comment-260378

Actually, make sure the bot never gets stuck in a loop. We have to a figure out a way which is much more reliable. Here's a proposal:
Process comments as usual (just fix the bug). But at the end of processing, when a new comment should be sent (note: applies to comments only, not ticket description updates), check the last 2 comments in the ticket. If both of them are from blockerbot, log it as a warning/error and don't send the comment. This will prevent loops. It should be extremely rare to have a valid scenario where the last 2 comments are from blockerbot and another one should be added. It would involve several very fast subsequent AGREED commands as separate comments, which would spawn several blockerbot handling threads in BBA, each of them seeing the ticket in a different state. If that unlikely scenario occurs, all we need is to anyone to add a comment (any value) on the ticket, and it will get fully processed again, appending remaining comments as needed.

[✓] For voting and admin commands, make sure people can use arbitrary amount of whitespace between keywords, not just a single space. Otherwise people will mistype, put in two spaces instead of one, and then will not understand why the bot is not doing anything. This also involves whitespace at the end of the line. So both these should ideally work:
"FinalBlocker +1"
"FinalBlocker +1 "
People will do this often without realizing, and we have no indication for incorrect commands, so we must at least consume extra whitespace.

Note: Make sure we process it line by line, so that this is not valid:
"FinalBlocker
+1"

During blocker meeting , sometimes we use punt for more information.. in that case what will be potential voting syntax? Also,punting also goes will a NEEDINFO.
I am referring to https://fedoraproject.org/wiki/QA:SOP_Blocker_Bug_Meeting#Proposed_blocker_.2F_freeze_exception_issue_not_accepted_or_rejected_.28.27punted.27.29

During blocker meeting , sometimes we use punt for more information.. in that case what will be potential voting syntax? Also,punting also goes will a NEEDINFO.
I am referring to https://fedoraproject.org/wiki/QA:SOP_Blocker_Bug_Meeting#Proposed_blocker_.2F_freeze_exception_issue_not_accepted_or_rejected_.28.27punted.27.29

I don't think we need any punt syntax here, there is no reason to punt, you just simply don't vote until you know all the information you need for voting.

Yes, there's no need for such command. You can write it as a comment "let's wait for xxx", or we can use some tag like "needinfo" on the ticket, if we prefer. It is expected that the person closing the vote with AGREED should be aware of the state of the ticket and not close the vote when it's not ready yet.

Metadata Update from @adamwill:
- Issue tagged with: enhancement, pagure

a year ago

The review discussions feature is now LIVE at https://pagure.io/fedora-qa/blocker-review . I've went through all the comments here and checked that all features/bugs mentioned here should be resolved. I believe we can close this.

Metadata Update from @kparal:
- Issue close_status updated to: Fixed
- Issue status updated to: Closed (was: Open)

a year ago

Login to comment on this ticket.

Metadata