From a8b2b1c7b38fadfeb5a2ff64193c4a735df4a5d0 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 21 Jun 2019 13:07:44 +0200 Subject: [PATCH 1/7] publish_toolstate: don't use 'new' from inside the loop --- src/tools/publish_toolstate.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index 9e7c18b7f566b..97df3cfda006d 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -135,13 +135,13 @@ def update_latest( for status in latest: tool = status['tool'] changed = False - create_issue = False + create_issue_for_status = None # set to the status that caused the issue for os, s in current_status.items(): old = status[os] new = s.get(tool, old) status[os] = new - if new > old: + if new > old: # comparing the strings, but they are ordered appropriately! # things got fixed or at least the status quo improved changed = True message += '🎉 {} on {}: {} → {} (cc {}, @rust-lang/infra).\n' \ @@ -156,12 +156,12 @@ def update_latest( # Most tools only create issues for build failures. # Other failures can be spurious. if new == 'build-fail' or (tool == 'miri' and new == 'test-fail'): - create_issue = True + create_issue_for_status = new - if create_issue: + if create_issue_for_status is not None: try: issue( - tool, new, MAINTAINERS.get(tool, ''), + tool, create_issue_for_status, MAINTAINERS.get(tool, ''), relevant_pr_number, relevant_pr_user, pr_reviewer, ) except IOError as e: From fb2f841867208fd155caff8af4d47b4067f7f845 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 21 Jun 2019 13:40:57 +0200 Subject: [PATCH 2/7] give a bit more context in the error message --- src/tools/publish_toolstate.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index 97df3cfda006d..13ab1dfeaa889 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -167,9 +167,9 @@ def update_latest( except IOError as e: # network errors will simply end up not creating an issue, but that's better # than failing the entire build job - print("I/O error: {0}".format(e)) + print("I/O error when creating issue for status regression: {0}".format(e)) except: - print("Unexpected error: {0}".format(sys.exc_info()[0])) + print("Unexpected error when creating issue for status regression: {0}".format(sys.exc_info()[0])) raise if changed: From e02c65519459f1f5d8309d0a6f4ada42be1be4e3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 21 Jun 2019 13:46:46 +0200 Subject: [PATCH 3/7] dump the JSON we are going to submit to GH to create the issue --- src/tools/publish_toolstate.py | 40 ++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index 13ab1dfeaa889..b27f2bb9f4374 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -77,27 +77,29 @@ def issue( status_description = 'has failing tests' else: status_description = 'no longer builds' + request = json.dumps({ + 'body': maybe_delink(textwrap.dedent('''\ + Hello, this is your friendly neighborhood mergebot. + After merging PR {}, I observed that the tool {} {}. + A follow-up PR to the repository {} is needed to fix the fallout. + + cc @{}, do you think you would have time to do the follow-up work? + If so, that would be great! + + cc @{}, the PR reviewer, and @rust-lang/compiler -- nominating for prioritization. + + ''').format( + relevant_pr_number, tool, status_description, + REPOS.get(tool), relevant_pr_user, pr_reviewer + )), + 'title': '`{}` no longer builds after {}'.format(tool, relevant_pr_number), + 'assignees': assignees, + 'labels': ['T-compiler', 'I-nominated'], + }) + print("Creating issue:\n{}".format(request)) response = urllib2.urlopen(urllib2.Request( gh_url(), - json.dumps({ - 'body': maybe_delink(textwrap.dedent('''\ - Hello, this is your friendly neighborhood mergebot. - After merging PR {}, I observed that the tool {} {}. - A follow-up PR to the repository {} is needed to fix the fallout. - - cc @{}, do you think you would have time to do the follow-up work? - If so, that would be great! - - cc @{}, the PR reviewer, and @rust-lang/compiler -- nominating for prioritization. - - ''').format( - relevant_pr_number, tool, status_description, - REPOS.get(tool), relevant_pr_user, pr_reviewer - )), - 'title': '`{}` no longer builds after {}'.format(tool, relevant_pr_number), - 'assignees': assignees, - 'labels': ['T-compiler', 'I-nominated'], - }), + request, { 'Authorization': 'token ' + github_token, 'Content-Type': 'application/json', From 8ceab3218ed277ec822e72cde9b06bfd53549dc2 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 21 Jun 2019 14:07:20 +0200 Subject: [PATCH 4/7] fix long line --- src/tools/publish_toolstate.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index b27f2bb9f4374..5c6d91a9c432c 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -171,7 +171,8 @@ def update_latest( # than failing the entire build job print("I/O error when creating issue for status regression: {0}".format(e)) except: - print("Unexpected error when creating issue for status regression: {0}".format(sys.exc_info()[0])) + print("Unexpected error when creating issue for status regression: {0}" + .format(sys.exc_info()[0])) raise if changed: From e10012d037a124efd6cde026d3562f7bb7b1dbf6 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 21 Jun 2019 19:22:46 +0200 Subject: [PATCH 5/7] show HTTP error body --- src/tools/publish_toolstate.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index 5c6d91a9c432c..9f196a219cc23 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -166,9 +166,11 @@ def update_latest( tool, create_issue_for_status, MAINTAINERS.get(tool, ''), relevant_pr_number, relevant_pr_user, pr_reviewer, ) - except IOError as e: + except urllib2.HTTPError as e: # network errors will simply end up not creating an issue, but that's better # than failing the entire build job + print("HTTPError when creating issue for status regression: {0}\n{1}".format(e, e.read())) + except IOError as e: print("I/O error when creating issue for status regression: {0}".format(e)) except: print("Unexpected error when creating issue for status regression: {0}" From ecca52cac14c28af3584ea531e3f4e0a15dfca67 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 21 Jun 2019 20:25:37 +0200 Subject: [PATCH 6/7] don't make PR author assignee; that break creating the issue when they are not a team member --- src/tools/publish_toolstate.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index 9f196a219cc23..ae4c12a94ab5d 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -72,7 +72,6 @@ def issue( ): # Open an issue about the toolstate failure. assignees = [x.strip() for x in maintainers.split('@') if x != ''] - assignees.append(relevant_pr_user) if status == 'test-fail': status_description = 'has failing tests' else: From 02863a3554a1b0aea93871bbfb7a92738cd2fe89 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 21 Jun 2019 20:47:42 +0200 Subject: [PATCH 7/7] do as tidy says --- src/tools/publish_toolstate.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index ae4c12a94ab5d..d5dff1dcae0d5 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -168,7 +168,8 @@ def update_latest( except urllib2.HTTPError as e: # network errors will simply end up not creating an issue, but that's better # than failing the entire build job - print("HTTPError when creating issue for status regression: {0}\n{1}".format(e, e.read())) + print("HTTPError when creating issue for status regression: {0}\n{1}" + .format(e, e.read())) except IOError as e: print("I/O error when creating issue for status regression: {0}".format(e)) except: