From 47a3dc5cd62da11d921996d923a6e28f114d6038 Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Nov 22 2023 20:03:30 +0000 Subject: [PATCH 1/2] fedora_bz: build in queries, get EOL date from Bodhi This has two QoL improvements for fedora_bz. The major one is that instead of having you run Bugzilla queries yourself, save the results as CSV files, and feed them in, it now can do the relevant queries itself, including handling pagination. The other is that the EOL functions now just get the EOL date from Bodhi if it's not passed in on the command line. There's a few other little cleanups mixed in, like combining the very similar functions that backed eolwarn and eolclose into a single one. Signed-off-by: Adam Williamson --- diff --git a/closebugs/fedora_bz.py b/closebugs/fedora_bz.py index e8d607e..fbbb24a 100755 --- a/closebugs/fedora_bz.py +++ b/closebugs/fedora_bz.py @@ -18,7 +18,7 @@ import argparse import bugzilla import jinja2 from urllib.error import HTTPError -import pandas +import requests # Define the return code types ERROR_CODES = { @@ -78,6 +78,43 @@ active release. Thank you for reporting this bug and we are sorry it could not be fixed.''' +BRANCH_QUERY = "https://bugzilla.redhat.com/buglist.cgi?classification=Fedora&f1=component&f3=component&f4=bug_status&f5=component&f6=component&keywords=FutureFeature%2C%20Tracking%2C%20&keywords_type=nowords&list_id=10832664&o1=notequals&o3=notequals&o4=notequals&o5=notequals&o6=notequals&product=Fedora&product=Fedora%20Container%20Images&query_format=advanced&short_desc=RFE&short_desc_type=notregexp&v1=Package%20Review&v3=kernel&v4=CLOSED&v5=Changes%20Tracking&v6=Container%20Review&version=rawhide" +EOL_QUERY = "https://bugzilla.redhat.com/buglist.cgi?bug_status=__open__&keywords=Tracking%2C%20&keywords_type=nowords&product=Fedora&product=Fedora%20Container%20Images&query_format=advanced" + + +def paginate_query(query, bz): + """ + Handle Bugzilla result pagination. Based on + https://github.com/python-bugzilla/python-bugzilla/issues/149#issuecomment-929966694 + but simplified (assumes we're always using advanced queries with + AND behaviour). + """ + bugs = [] + query["limit"] = 0 + query["order_by"] = "bug_id" + + field = max([int(f[1:]) for f in query.keys() if f.startswith("f")] or [0]) + 1 + + query[f"f{field}"] = "bug_id" + query[f"o{field}"] = "greaterthan" + query[f"v{field}"] = 0 + + while True: + result = bz.query(query) + bugs.extend(result) + count = len(result) + + if count == 0: + break + + query[f"v{field}"] = bugs[-1].id + + return bugs + + +def guess_eol_date(version): + return requests.get(f"https://bodhi.fedoraproject.org/releases/F{version}").json()["eol"] + def error_out(message, ret_code=1): ''' @@ -86,46 +123,8 @@ def error_out(message, ret_code=1): print("ERROR! %s" % message, file=sys.stderr) sys.exit(ret_code) -def get_bugs(): - ''' - Read bugs from a CSV file and return the IDs - ''' - try: - bug_data = pandas.read_csv(options.filename) - except FileNotFoundError: - error_out('File %s not found' % options.filename, ERROR_CODES['FILE']) - except pandas.errors.EmptyDataError: - error_out('Could not parse file %s' % options.filename, ERROR_CODES['FILE']) - - # Change some column names because of RHBZ 2033441 - col_rename_map = {} - if 'Vers ' in list(bug_data.columns): - col_rename_map['Vers '] = 'Version' - if 'ID' in list(bug_data.columns): - col_rename_map['ID'] = 'Bug ID' - if col_rename_map: - print("Remapping columns: %s" % col_rename_map) - bug_data.rename(columns=col_rename_map, inplace=True) - else: - print("No columns to rename") - - # Check that the version is what we expect - if options.check_version: - print("Checking for version %s" % options.check_version) - # Force the version field to a string since we might have numbers or "rawhide" - try: - bug_data = bug_data.astype({'Version': str}) - mismatch_count = len(bug_data[bug_data['Version'] != options.check_version]) - except KeyError: - error_out("No version field in the CSV file", ERROR_CODES['DATA']) - - if mismatch_count > 0: - error_out("%i bugs do not match specified version %s" % \ - (mismatch_count, options.check_version), ERROR_CODES['DATA']) - return bug_data['Bug ID'].tolist() - -def branch_bugs(bugs): +def branch_bugs(): '''Branch the bugs to the new release''' # Check to make sure we have a new version specifed @@ -140,14 +139,23 @@ def branch_bugs(bugs): comment = jinja2.Environment().from_string(BRANCH_TEMPLATE).render(\ VERSION=options.new_version, PRODUCT=options.product) + query = bz.url_to_query(BRANCH_QUERY) + query["include_fields"] = ["id"] + if options.date: + # this is for specifying the Branch date if the script is run + # late, so we don't touch bugs filed after the branch date + query["chfield"] = "[Bug creation]" + query["chfieldto"] = options.date + bugs = [bug.id for bug in paginate_query(query, bz)] + # options.mail = 0 update = bz.build_update(comment=comment, version=options.new_version, \ minor_update=options.mail) update_bugs(bugs, update) -def alert_deadline(bugs, milestone): - '''Comment on Changes tracking bugs that missed the testable completion deadline''' +def alert_deadline(milestone): + '''Comment on Changes tracking bugs that missed a deadline''' if not options.new_version or \ (milestone == "testable" and not options.date): @@ -157,10 +165,18 @@ def alert_deadline(bugs, milestone): match milestone: case "testable": template = TESTABLE_TEMPLATE + status = ["NEW", "ASSIGNED", "POST"] case "complete": template = COMPLETE_TEMPLATE + status = ["NEW", "ASSIGNED", "POST", "MODIFIED"] case _: - error_out("Whoa! How did milestone %s get here?!" % milestone, ERROR_CODES['SCRIPT']) + error_out(f"Whoa! How did milestone {milestone} get here?!", ERROR_CODES['SCRIPT']) + + query = bz.build_query(status=status, blocked=f"F{options.new_version}Changes") + query["include_fields"] = ["id"] + query["limit"] = 0 + # we don't paginate this one as it really shouldn't ever hit the limit + bugs = [bug.id for bug in bz.query(query)] # If we didn't explictly specify a preference for needinfo, then set it to true for this if options.needinfo is None: @@ -173,29 +189,41 @@ def alert_deadline(bugs, milestone): update_bugs(bugs, update) -def warn_eol(bugs): - '''Post the EOL warning''' - - if not options.new_version or not options.date: - error_out("This command requires --release and --date", ERROR_CODES['SCRIPT']) - - comment = jinja2.Environment().from_string(EOL_WARN_TEMPLATE).render(\ - VERSION=options.new_version,PRODUCT=options.product, DATE=options.date) - - update = bz.build_update(comment=comment, minor_update=options.mail) - update_bugs(bugs, update) - -def close_eol(bugs): - '''Close EOL bugs''' - - if not options.new_version or not options.date: - error_out("This command requires --release and --date", ERROR_CODES['SCRIPT']) - - comment = jinja2.Environment().from_string(EOL_CLOSE_TEMPLATE).render(\ - VERSION=options.new_version,PRODUCT=options.product, DATE=options.date) +def handle_eol(close=False): + ''' + Handle EOL (post the warning if close is False, close bugs if + close is True) + ''' - update = bz.build_update(comment=comment, minor_update=options.mail, \ - status="CLOSED", resolution="EOL") + if not options.new_version: + error_out("This command requires --release", ERROR_CODES['SCRIPT']) + + date = options.date or guess_eol_date(options.new_version) + + query = bz.url_to_query(EOL_QUERY) + query["include_fields"] = ["id"] + query["version"] = options.new_version + bugs = [bug.id for bug in paginate_query(query, bz)] + + if close: + comment = jinja2.Environment().from_string(EOL_CLOSE_TEMPLATE).render( + VERSION=options.new_version, + PRODUCT=options.product, + DATE=options.date + ) + update = bz.build_update( + comment=comment, + minor_update=options.mail, + status="CLOSED", + resolution="EOL" + ) + else: + comment = jinja2.Environment().from_string(EOL_WARN_TEMPLATE).render( + VERSION=options.new_version, + PRODUCT=options.product, + DATE=date + ) + update = bz.build_update(comment=comment, minor_update=options.mail) update_bugs(bugs, update) @@ -276,29 +304,24 @@ parser.add_argument('--sleep-every', dest='sleep_every', type=int, default=10, parser.add_argument('--sleep-seconds', dest='sleep_seconds', type=int, default=2, help='Sleep period (in seconds)') parser.add_argument('command', type=str, help='Bug operation to perform (branch,)') -parser.add_argument('filename', type=str, help='CSV file with bugs') options = parser.parse_args() -# Get the list of bugs we're going to act on -bug_list = get_bugs() - -# Setup Bugizlla +# Setup Bugzilla bz = bugzilla.Bugzilla(options.bz_server) if not bz.logged_in: error_out('Not logged in to Bugzilla server at %s' % options.bz_server, ERROR_CODES['BZ']) - # Now do...whatever it is we're going to do match options.command: case "branch": - branch_bugs(bug_list) + branch_bugs() case "deadline-testable": - alert_deadline(bug_list, "testable") + alert_deadline("testable") case "deadline-complete": - alert_deadline(bug_list, "complete") + alert_deadline("complete") case "eolwarn": - warn_eol(bug_list) + handle_eol(close=False) case "eolclose": - close_eol(bug_list) + handle_eol(close=True) case _: error_out("Unknown command %s" % options.command, ERROR_CODES['SCRIPT']) From 825d66aa448aae261f8f3a44d2fdb49758023e2d Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Nov 22 2023 20:03:33 +0000 Subject: [PATCH 2/2] Use argparse more extensively This uses argparse's capabilities more extensively to provide somewhat better handling of required and optional arguments for each command (now implemented as argparse subcommands), and somewhat better descriptions. Signed-off-by: Adam Williamson --- diff --git a/closebugs/fedora_bz.py b/closebugs/fedora_bz.py index fbbb24a..6f38294 100755 --- a/closebugs/fedora_bz.py +++ b/closebugs/fedora_bz.py @@ -22,7 +22,6 @@ import requests # Define the return code types ERROR_CODES = { - 'SCRIPT': 2, 'FILE' : 3, 'DATA': 4, 'BZ': 5 @@ -116,6 +115,13 @@ def guess_eol_date(version): return requests.get(f"https://bodhi.fedoraproject.org/releases/F{version}").json()["eol"] +def check_date(datestr): + '''Check a string is a valid YYYY-MM-DD date.''' + # if it isn't, this will raise an error argparse will handle + datetime.strptime(datestr, "%Y-%m-%d") + return datestr + + def error_out(message, ret_code=1): ''' A small function to print an error to stderr and then quit. @@ -127,17 +133,13 @@ def error_out(message, ret_code=1): def branch_bugs(): '''Branch the bugs to the new release''' - # Check to make sure we have a new version specifed - if not options.new_version: - error_out("No new version specified for branching.", ERROR_CODES['SCRIPT']) - # If the user didn't explicitly request email, let's disable it now. # The Bugzilla API can handle getting a 'None' otherwise. if options.mail is None: options.mail = 1 comment = jinja2.Environment().from_string(BRANCH_TEMPLATE).render(\ - VERSION=options.new_version, PRODUCT=options.product) + VERSION=options.release, PRODUCT=options.product) query = bz.url_to_query(BRANCH_QUERY) query["include_fields"] = ["id"] @@ -149,30 +151,24 @@ def branch_bugs(): bugs = [bug.id for bug in paginate_query(query, bz)] # options.mail = 0 - update = bz.build_update(comment=comment, version=options.new_version, \ + update = bz.build_update(comment=comment, version=options.release, \ minor_update=options.mail) update_bugs(bugs, update) -def alert_deadline(milestone): +def alert_deadline(): '''Comment on Changes tracking bugs that missed a deadline''' + if options.testable: + template = TESTABLE_TEMPLATE + date = options.date + status = ["NEW", "ASSIGNED", "POST"] + else: + template = COMPLETE_TEMPLATE + status = ["NEW", "ASSIGNED", "POST", "MODIFIED"] + # not actually used in the COMPLETE template + date = None - if not options.new_version or \ - (milestone == "testable" and not options.date): - error_out("This command requires --release and (for testable) --date", \ - ERROR_CODES['SCRIPT']) - - match milestone: - case "testable": - template = TESTABLE_TEMPLATE - status = ["NEW", "ASSIGNED", "POST"] - case "complete": - template = COMPLETE_TEMPLATE - status = ["NEW", "ASSIGNED", "POST", "MODIFIED"] - case _: - error_out(f"Whoa! How did milestone {milestone} get here?!", ERROR_CODES['SCRIPT']) - - query = bz.build_query(status=status, blocked=f"F{options.new_version}Changes") + query = bz.build_query(status=status, blocked=f"F{options.release}Changes") query["include_fields"] = ["id"] query["limit"] = 0 # we don't paginate this one as it really shouldn't ever hit the limit @@ -182,32 +178,29 @@ def alert_deadline(milestone): if options.needinfo is None: options.needinfo = True - comment = jinja2.Environment().from_string(template).render(VERSION=options.new_version, \ - PRODUCT=options.product, DATE=options.date) + comment = jinja2.Environment().from_string(template).render(VERSION=options.release, \ + PRODUCT=options.product, DATE=date) update = bz.build_update(comment=comment, minor_update=options.mail) update_bugs(bugs, update) -def handle_eol(close=False): +def handle_eol(): ''' - Handle EOL (post the warning if close is False, close bugs if - close is True) + Handle EOL (post the warning if options.close is False, close bugs + if options.close is True) ''' - if not options.new_version: - error_out("This command requires --release", ERROR_CODES['SCRIPT']) - - date = options.date or guess_eol_date(options.new_version) + date = options.date or guess_eol_date(options.release) query = bz.url_to_query(EOL_QUERY) query["include_fields"] = ["id"] - query["version"] = options.new_version + query["version"] = options.release bugs = [bug.id for bug in paginate_query(query, bz)] - if close: + if options.close: comment = jinja2.Environment().from_string(EOL_CLOSE_TEMPLATE).render( - VERSION=options.new_version, + VERSION=options.release, PRODUCT=options.product, DATE=options.date ) @@ -219,7 +212,7 @@ def handle_eol(close=False): ) else: comment = jinja2.Environment().from_string(EOL_WARN_TEMPLATE).render( - VERSION=options.new_version, + VERSION=options.release, PRODUCT=options.product, DATE=date ) @@ -286,24 +279,69 @@ def update_bugs(bugs, update): parser = argparse.ArgumentParser(description='A Bugzilla mass-updater for Fedora') parser.add_argument('--server', dest='bz_server', type=str, default='bugzilla.redhat.com', help='Bugzilla server (default: bugzilla.redhat.com)') -parser.add_argument('--check-version', dest='check_version', type=str, - help='Check that the verison matches a given string') -parser.add_argument('--disable-mail', dest='mail', action=argparse.BooleanOptionalAction, \ +parser.add_argument('--disable-mail', dest='mail', action=argparse.BooleanOptionalAction, help='Disable email notification') -parser.add_argument('--date', dest='date', type=str, help='Date to use in comments') parser.add_argument('--dry-run', dest='dry_run', action='store_true', help='Do not perform Bugzilla update calls') -parser.add_argument('--needinfo', dest='needinfo', action=argparse.BooleanOptionalAction, \ +parser.add_argument('--needinfo', dest='needinfo', action=argparse.BooleanOptionalAction, help='Set needinfo on appropriate actions') -parser.add_argument('--product', dest='product', type=str, default="Fedora Linux", \ +parser.add_argument('--product', dest='product', type=str, default="Fedora Linux", help='Product name to use in text (default: Fedora Linux)') -parser.add_argument('--release', dest='new_version', type=int, \ - help='Version for branching and bug comments') parser.add_argument('--sleep-every', dest='sleep_every', type=int, default=10, help='Sleep after every X bugs') parser.add_argument('--sleep-seconds', dest='sleep_seconds', type=int, default=2, help='Sleep period (in seconds)') -parser.add_argument('command', type=str, help='Bug operation to perform (branch,)') + +subparsers = parser.add_subparsers(dest="subcommand") +subparsers.required = True + +parser_branch = subparsers.add_parser( + "branch", + description="Change bug versions from rawhide to the new release at Branch point" +) +parser_branch.add_argument('--date', type=check_date, + help='The branch date (YYYY-MM-DD) (specify if running script late)') +parser_branch.set_defaults(func=branch_bugs) + +parser_deadlinet = subparsers.add_parser( + "deadline-testable", + description="Comment on Change bugs that have not reached MODIFIED at the testable deadline" +) +parser_deadlinet.add_argument( + 'date', + type=check_date, + help='The 100%% complete deadline date (*NOT* the testable date!), included in the comment' +) +parser_deadlinet.set_defaults(func=alert_deadline, testable=True) + +parser_deadlinec = subparsers.add_parser( + "deadline-complete", + description="Comment on Change bugs that have not reached ON_QA at the complete deadline" +) +parser_deadlinec.set_defaults(func=alert_deadline, testable=False) + +parser_eolw = subparsers.add_parser( + "eolwarn", + description="Comment on bugs that will soon be closed for a release that is going EOL" +) +parser_eolw.set_defaults(func=handle_eol, close=False) + +parser_eolc = subparsers.add_parser( + "eolclose", + description="Close bugs when release goes EOL" +) +parser_eolc.set_defaults(func=handle_eol, close=True) + +for subp in (parser_branch, parser_deadlinet, parser_deadlinec, parser_eolw, parser_eolc): + subp.add_argument('release', help='The release to work on') + +for eolp in (parser_eolw, parser_eolc): + eolp.add_argument( + '--date', + type=check_date, + help='The EOL date (will be read from Bodhi if not specified)' + ) + options = parser.parse_args() # Setup Bugzilla @@ -312,16 +350,4 @@ if not bz.logged_in: error_out('Not logged in to Bugzilla server at %s' % options.bz_server, ERROR_CODES['BZ']) # Now do...whatever it is we're going to do -match options.command: - case "branch": - branch_bugs() - case "deadline-testable": - alert_deadline("testable") - case "deadline-complete": - alert_deadline("complete") - case "eolwarn": - handle_eol(close=False) - case "eolclose": - handle_eol(close=True) - case _: - error_out("Unknown command %s" % options.command, ERROR_CODES['SCRIPT']) +options.func()