Skip to content

Fix update-checkout error reporting #60234

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 44 additions & 26 deletions utils/update_checkout/update_checkout/update_checkout.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,18 +60,33 @@ def check_parallel_results(results, op):
parallel implementation.
"""

fail_count = 0
if results is None:
return 0

# Remove non-errors
results = [r for r in results if r]

if results:
print(f"======{op} FAILURES======", file=sys.stderr)

messages = []
for r in results:
if r is not None:
if fail_count == 0:
print("======%s FAILURES======" % op)
print("%s failed (ret=%d): %s" % (r.repo_path, r.ret, r))
fail_count += 1
if r.stderr:
print(r.stderr)
return fail_count
try:
raise r
except KeyError as e:
messages.append(f"Failed to look up {e} in scheme\n")
except Exception:
Comment on lines +74 to +78
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized you can make this a bit simpler and avoid the "raise for pattern matching" which is a bit quirky

Suggested change
try:
raise r
except KeyError as e:
messages.append(f"Failed to look up {e} in scheme\n")
except Exception:
if isinstance(r, KeyError):
messages.append(f"Failed to look up {r} in scheme\n")
else:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure which read better. It should probably be single JobError with a __str__ method that just prints the right thing so this can just iterate over the list and print it without having to figure out what happened in the job that failed (even the KeyError here could lead to a misleading error message), but that is a bigger PR.

path = getattr(r, 'repo_path', '')
reason = getattr(r, 'stderr', '')
if path:
message = f"{path} failed"
if reason:
message += f"\n```\n{reason}\n```"
message += f"\n{r}"
if message:
messages.append(message + '\n')
print('---\n\n'.join(messages), file=sys.stderr)
return len(results)


def confirm_tag_in_repo(tag, repo_name):
Expand Down Expand Up @@ -100,20 +115,24 @@ def get_branch_for_repo(config, repo_name, scheme_name, scheme_map,
cross_repo = False
repo_branch = scheme_name
if scheme_map:
scheme_branch = scheme_map[repo_name]
repo_branch = scheme_branch
remote_repo_id = config['repos'][repo_name]['remote']['id']
if remote_repo_id in cross_repos_pr:
cross_repo = True
pr_id = cross_repos_pr[remote_repo_id]
repo_branch = "ci_pr_{0}".format(pr_id)
shell.run(["git", "checkout", scheme_branch],
echo=True)
shell.capture(["git", "branch", "-D", repo_branch],
echo=True, allow_non_zero_exit=True)
shell.run(["git", "fetch", "origin",
"pull/{0}/merge:{1}"
.format(pr_id, repo_branch), "--tags"], echo=True)
try:
scheme_branch = scheme_map[repo_name]
repo_branch = scheme_branch
remote_repo_id = config['repos'][repo_name]['remote']['id']
if remote_repo_id in cross_repos_pr:
cross_repo = True
pr_id = cross_repos_pr[remote_repo_id]
repo_branch = "ci_pr_{0}".format(pr_id)
shell.run(["git", "checkout", scheme_branch],
echo=True)
shell.capture(["git", "branch", "-D", repo_branch],
echo=True, allow_non_zero_exit=True)
shell.run(["git", "fetch", "origin",
"pull/{0}/merge:{1}"
.format(pr_id, repo_branch), "--tags"], echo=True)
except KeyError:
print(f"Failed to look up {repo_name} in scheme", file=sys.stderr)
raise
return repo_branch, cross_repo


Expand Down Expand Up @@ -634,10 +653,9 @@ def main():

update_results = update_all_repositories(args, config, scheme,
cross_repos_pr)
fail_count = 0
fail_count += check_parallel_results(clone_results, "CLONE")
fail_count = check_parallel_results(clone_results, "CLONE")
fail_count += check_parallel_results(update_results, "UPDATE")
if fail_count > 0:
if fail_count:
print("update-checkout failed, fix errors and try again")
else:
print("update-checkout succeeded")
Expand Down