Merge lp:~sil2100/ubuntu-archive-tools/sru-review-invalid-bugs into lp:ubuntu-archive-tools

[Łukasz Zemczak]

Propose not excluding tasks that are in 'invalid/released' states when handling sru-review bugs as otherwise the tool can crash trying to re-add a task for an existing suite.

Propose not excluding tasks that are in 'invalid/released' states when handling sru-review bugs as otherwise the tool can crash trying to re-add a task for an existing suite.

I have decided that printing a warning is enough as this step happens after the SRU has been already accepted from the queue, so prompting for reviewer input is at this moment meaningless. A reviewer knows what she/he is doing and otherwise the tool crash can just cause the SRU bugs not being handled at all.

[Steve Langasek (vorlon)]

I don't like the idea of having the review tool automatically setting such bug tasks back to fix-committed; this seems like an error on the input side which should be resolved prior to accepting the SRU, because the correct resolution varies. (Sometimes this means the bug task should be reopened, but sometimes it indicates a wrong bug is linked from the SRU and the tool shouldn't presume.)

review: Needs Fixing

[Brian Murray (brian-murray)]

This seems fine to me.

review: Approve

[Łukasz Zemczak (sil2100)]

Steve: I agree it might somehow be a problem, but leaving it as is right now is breaks the whole tool and makes the reviewer's life really hard (as sru-review crashes after accepting and no bugs get updated and need to be updated manually). The best solution would be of course to check the bugs before actually accepting the SRU, but this means a bigger rewrite of the code. My assumption is that the SRU reviewers responsibility is to make sure the package is good for -proposed promotion before actually accepting, during the review - so the status is just a formality that was left untouched.

[Steve Langasek (vorlon)]

ok, objections withdrawn

review: Approve

[Brian Murray (brian-murray)]

Given Steve's concerns the print statement should be more informative about the fact that the reviewer should double check the bug for any errors.

[lp:~sil2100/ubuntu-archive-tools/sru-review-invalid-bugs]

1160. By Łukasz Zemczak on 2018-03-06

Make the warning more verbose.

[Łukasz Zemczak (sil2100)]

Made more informative. Thanks!

=== modified file 'sru_workflow.py'
--- sru_workflow.py	2017-12-07 16:28:15 +0000
+++ sru_workflow.py	2018-03-06 08:52:48 +0000
@@ -38,9 +38,11 @@
             print("Ignoring task %s in bug %s" % (task.web_link, num))
             continue
         sourcepkg_match = True
-        if (match.group('suite') == release and
-            task.status not in ("Invalid", "Won't Fix",
-                                "Fix Released")):
+        if match.group('suite') == release:
+            if task.status in ("Invalid", "Won't Fix", "Fix Released"):
+                print("Matching task was set to %s before accepting the SRU, "
+                      "please double-check if this bug is still liable for "
+                      "fixing.  Switching to Fix Committed." % task.status)
             task.status = "Fix Committed"
             task.lp_save()
             print("Success: task %s in bug %s" % (task.web_link, num))