Skip to content

mercurial: refactor how we handle retries. - #102

Merged
abpostelnicu merged 1 commit into
masterfrom
rename-var
Jun 29, 2023
Merged

abpostelnicu merged 1 commit into
masterfrom
rename-var

Conversation

@abpostelnicu

Copy link
Copy Markdown
Contributor

This is just a refactor on how to handle retries for mercurial.

@abpostelnicu abpostelnicu self-assigned this Jun 29, 2023

@suhaibmujahid suhaibmujahid left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM! I guess reading error_log from the parent scope was causing problems. Thank you for fixing it.

Comment thread libmozevent/mercurial.py
Comment on lines +404 to +408
eligible_errors = [
"push failed on remote",
"stream ended unexpectedly",
"error: EOF occurred in violation of protocol",
]

@suhaibmujahid suhaibmujahid Jun 29, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: eligible_errors could be moved to be a class or module constant (could be named ELIGIBLE_RETRY_ERRORS)

Comment thread libmozevent/mercurial.py
Comment on lines +409 to +412
for eligible_message in eligible_errors:
if eligible_message in error:
return True
return False

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: since you are refactoring this part, using any() would be more pythonic

Suggested change
for eligible_message in eligible_errors:
if eligible_message in error:
return True
return False
return any(eligible_message in error for eligible_message in eligible_errors)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants