Skip to content
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

Merge retry logic into a single decorator #186

Merged
merged 7 commits into from
Jan 26, 2022
Merged

Merge retry logic into a single decorator #186

merged 7 commits into from
Jan 26, 2022

Conversation

ItsDrike
Copy link
Contributor

@ItsDrike ItsDrike commented Jan 3, 2022

Closes #139

As discussed in the issue, this introduces a retry decorator which will automate the logic of retrying the code for several specified amount of tries until the function was successful or if the function failed in each of the tries, raise the last exception that occurred.

  • Base retry decorator logic
  • Convert functions implementing the logic manually to use the decorator
  • Pass unittests
  • Add tests for the decorator

- Requiring simple Iterable type to pass the exceptable exceptions for
  the retry decorator wasn't viable since except can only take a type
  which inherits from BaseException or a tuple of these exception types.
- While this could also be done by converting the iterable into a tuple,
  that just brings additional overhead when the types could simply be
  passed in as a tuple to start with.
- Retrying connection creation and establishment isn't what we want to
  do in many cases.
- This is the reason tests for the server were failing!
- Even if it doesn't mean breakage, it's not worth to retry code that
  can only run once, it introduces unnecessary delays
- The downside to this is that the code looks a lot messier, but at
  least with some methods, it's absolutely necessary, as for the others,
  if this isn't desired it can be mentioned in the PR review.
@ItsDrike ItsDrike marked this pull request as ready for review January 6, 2022 01:20
@ItsDrike
Copy link
Contributor Author

ItsDrike commented Jan 6, 2022

The _retry_X methods that I've used are admittedly a bit weird to see and I'm opened for suggestions on better names or even completely different better approaches.

I've done this because we only want to repeat a certain part of those functions and it doesn't make sense to decorate the whole function and re-run everything in it. Not only would that be inefficient, it often wouldn't even be what's desired and may cause issues.

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.

Refactor duplicate retry handling code
2 participants