From ccff38ff87387defe1c8e9413d7453914d79dc03 Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Sep 13 2023 23:14:46 +0000 Subject: [PATCH 1/3] createbz: emails: drop parse_multi, do a bit more processing parse_multi is rather overcomplicated here - all we're actually doing with it is splitting a string on commas. I'm pretty sure we don't use any of the plus, minus or equals handling. So drop it and just do a comma split inline. Also take the opportunity to do a bit more parsing of the email addresses - replace another antispam obfuscation that fujiwarat uses, and strip spaces. Signed-off-by: Adam Williamson --- diff --git a/changes/createbz.py b/changes/createbz.py index b968811..eb7ae46 100644 --- a/changes/createbz.py +++ b/changes/createbz.py @@ -32,38 +32,6 @@ import time default_bz = 'https://bugzilla.redhat.com' -def _parse_triset(vallist, checkplus=True, checkminus=True, checkequal=True, - splitcomma=False): - add_val = [] - rm_val = [] - set_val = None - - def make_list(v): - if not v: - return [] - if splitcomma: - return v.split(",") - return [v] - - for val in type(vallist) is list and vallist or [vallist]: - val = val or "" - - if val.startswith("+") and checkplus: - add_val += make_list(val[1:]) - elif val.startswith("-") and checkminus: - rm_val += make_list(val[1:]) - elif val.startswith("=") and checkequal: - # Intentionally overwrite this - set_val = make_list(val[1:]) - else: - add_val += make_list(val) - - return add_val, rm_val, set_val - -def parse_multi(val): - return _parse_triset(val, checkplus=False, checkminus=False, - checkequal=False, splitcomma=True)[0] - if __name__ == "__main__": parser = OptionParser(usage="%prog INPUT", version="%prog 0.1") @@ -75,7 +43,16 @@ if __name__ == "__main__": reader = csv.DictReader(open(args[0], 'r')) #print("DEBUG %s" % list(reader)) for record in reader: - print(record["Name"], parse_multi(record["Email"].lower().replace("<", "").replace(">", "").replace(" at ", "@").replace(" dot ", "."))) + # normalize and replace antispam efforts + emails = record["Email"].lower().replace("<", "").replace(">", "").replace(" at ", "@").replace(" dot ", ".") + emails = emails.replace(" [at] ", "@").replace(" [dot] ", ".") + # split on commas + emails = emails.split(",") + # drop whitespace + emails = [email.strip() for email in emails] + # replace empty string with None, python-bugzilla needs this + emails = emails or None + print(record["Name"], emails) if record["Bug"] != "": print("Bug already exists") @@ -88,7 +65,7 @@ if __name__ == "__main__": description = "This is a tracking bug for Change: " + record["Name"] + \ "\nFor more details, see: " + record["Page"] + "\n\n" + record["Summary"] + \ "\n\nIf you encounter a bug related to this Change, please do not comment here. Instead create a new bug and set it to block this bug.", - cc = parse_multi(record["Email"].lower().replace("<", "").replace(">", "").replace(" at ", "@").replace(" dot ", ".")) or None, + cc = emails, ) print("Creating new bug") From 3b3c0a47896448031aaccc58fcfd3425cb05a899 Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Sep 14 2023 00:55:50 +0000 Subject: [PATCH 2/3] createbz: be safer with email addresses, assign the bug It's a known problem that the script blows up if any of the email addresses isn't registered in bugzilla (because it tries to include them all in the CC list). Let's improve that. First, we create the bug, with no CC list or assignee. Then, we try and add all the email addresses to the CC list and assign the bug to the first email in the list. If that works, great. If not, we try adding one address at a time to the CC list, catching the error if it doesn't work. If we managed to add *any* address to the CC list, we assign the bug to the *first* address that worked. I tried to be strategic about placing sleeps in this logic to make it not take *too* long, I think it should be safe to send the updates one after the other without sleeping. Signed-off-by: Adam Williamson --- diff --git a/changes/createbz.py b/changes/createbz.py index eb7ae46..03cecf9 100644 --- a/changes/createbz.py +++ b/changes/createbz.py @@ -29,6 +29,7 @@ import csv from optparse import OptionParser import bugzilla import time +import xmlrpc.client default_bz = 'https://bugzilla.redhat.com' @@ -50,8 +51,6 @@ if __name__ == "__main__": emails = emails.split(",") # drop whitespace emails = [email.strip() for email in emails] - # replace empty string with None, python-bugzilla needs this - emails = emails or None print(record["Name"], emails) if record["Bug"] != "": @@ -65,15 +64,44 @@ if __name__ == "__main__": description = "This is a tracking bug for Change: " + record["Name"] + \ "\nFor more details, see: " + record["Page"] + "\n\n" + record["Summary"] + \ "\n\nIf you encounter a bug related to this Change, please do not comment here. Instead create a new bug and set it to block this bug.", - cc = emails, ) print("Creating new bug") b = bz.createbug(ret) # BZ is too slow... time.sleep(5) + if emails: + # try adding CCs and setting assignee all in one go + try: + vals = bz.build_update(cc_add=emails, assigned_to=emails[0]) + bz.update_bugs([b.id], vals) + time.sleep(5) + except xmlrpc.client.Fault: + # probably means one of the emails is invalid, so let's go slow + # do the CCs, keeping note of successful ones + assignees = [] + for email in emails: + try: + vals = bz.build_update(cc_add=[email]) + bz.update_bugs([b.id], vals) + assignees.append(email) + except xmlrpc.client.Fault: + pass + # if we got any assignee candidates, do the assignment + # just because an address can be CCed doesn't mean it + # can be made the assignee - it seems accounts can be + # disabled, and you can CC disabled accounts but not + # assign bugs to them + if assignees: + for assignee in assignees: + try: + vals = bz.build_update(assigned_to=assignee) + bz.update_bugs([b.id], vals) + break + except xmlrpc.client.Fault: + pass + time.sleep(5) b.refresh() - print(b) print("") From d6e61b6eaf3cd1384be3d28a11a753c313f1cb65 Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Sep 14 2023 01:00:11 +0000 Subject: [PATCH 3/3] createbz: add another check for an existing bug If this script is run more than once without feature-pages.csv being updated - which can certainly happen if the script crashes or something - we will create duplicate bugs, which is not good. This adds a second check for an existing bug (with the same component, product, version and description) before creating a new bug. It'll make the script a bit slower, but I think it's best to be safe. It also tweaks the flow control a bit to avoid nesting the entire meat of the script in an `else` clause, instead we use `continue` to move on if we find an existing bug. Signed-off-by: Adam Williamson --- diff --git a/changes/createbz.py b/changes/createbz.py index 03cecf9..2fdde7a 100644 --- a/changes/createbz.py +++ b/changes/createbz.py @@ -55,53 +55,68 @@ if __name__ == "__main__": if record["Bug"] != "": print("Bug already exists") - else: - ret = bz.build_createbug( - component = "Changes Tracking", - product = "Fedora", - version = "rawhide", - summary = record["Name"], - description = "This is a tracking bug for Change: " + record["Name"] + \ - "\nFor more details, see: " + record["Page"] + "\n\n" + record["Summary"] + \ - "\n\nIf you encounter a bug related to this Change, please do not comment here. Instead create a new bug and set it to block this bug.", - ) + print("") + continue - print("Creating new bug") - b = bz.createbug(ret) - # BZ is too slow... - time.sleep(5) - if emails: - # try adding CCs and setting assignee all in one go - try: - vals = bz.build_update(cc_add=emails, assigned_to=emails[0]) - bz.update_bugs([b.id], vals) - time.sleep(5) - except xmlrpc.client.Fault: - # probably means one of the emails is invalid, so let's go slow - # do the CCs, keeping note of successful ones - assignees = [] - for email in emails: + # another check for an existing bug, in case this script is + # run twice without feature-pages.csv being updated + query = bz.build_query( + short_desc=record["Name"], + component="Changes Tracking", + product="Fedora" + ) + ret = bz.query(query) + if ret: + print("Bug already exists") + print(ret[0]) + print("") + continue + + ret = bz.build_createbug( + component = "Changes Tracking", + product = "Fedora", + version = "rawhide", + summary = record["Name"], + description = "This is a tracking bug for Change: " + record["Name"] + \ + "\nFor more details, see: " + record["Page"] + "\n\n" + record["Summary"] + \ + "\n\nIf you encounter a bug related to this Change, please do not comment here. Instead create a new bug and set it to block this bug.", + ) + + print("Creating new bug") + b = bz.createbug(ret) + # BZ is too slow... + time.sleep(5) + if emails: + # try adding CCs and setting assignee all in one go + try: + vals = bz.build_update(cc_add=emails, assigned_to=emails[0]) + bz.update_bugs([b.id], vals) + time.sleep(5) + except xmlrpc.client.Fault: + # probably means one of the emails is invalid, so let's go slow + # do the CCs, keeping note of successful ones + assignees = [] + for email in emails: + try: + vals = bz.build_update(cc_add=[email]) + bz.update_bugs([b.id], vals) + assignees.append(email) + except xmlrpc.client.Fault: + pass + # if we got any assignee candidates, do the assignment + # just because an address can be CCed doesn't mean it + # can be made the assignee - it seems accounts can be + # disabled, and you can CC disabled accounts but not + # assign bugs to them + if assignees: + for assignee in assignees: try: - vals = bz.build_update(cc_add=[email]) + vals = bz.build_update(assigned_to=assignee) bz.update_bugs([b.id], vals) - assignees.append(email) + break except xmlrpc.client.Fault: pass - # if we got any assignee candidates, do the assignment - # just because an address can be CCed doesn't mean it - # can be made the assignee - it seems accounts can be - # disabled, and you can CC disabled accounts but not - # assign bugs to them - if assignees: - for assignee in assignees: - try: - vals = bz.build_update(assigned_to=assignee) - bz.update_bugs([b.id], vals) - break - except xmlrpc.client.Fault: - pass - time.sleep(5) - b.refresh() - print(b) - + time.sleep(5) + b.refresh() + print(b) print("")