-
Notifications
You must be signed in to change notification settings - Fork 8
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
add retry mechanism to write_backlog #605
add retry mechanism to write_backlog #605
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the contribution. Please have a look for my comments on your implementation
def _parallel_bulk_with_retries(self, client, actions, *args, **kwargs): | ||
bulk_delays = 1 | ||
for _ in range(self._config.bulk_retries): | ||
try: | ||
self._parallel_bulk(actions, client, *args, **kwargs) | ||
return | ||
except search.ConnectionError as error: | ||
raise error | ||
except search.exceptions.TransportError as error: | ||
if self._message_exceeds_max_size_error(error): | ||
raise error | ||
time.sleep(bulk_delays) | ||
bulk_delays += random.randint(500, 1000) / 1000 | ||
raise FatalOutputError( | ||
self, "Opensearch too many requests, all parallel bulk retries failed" | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of writing this new method, how about extending the called method and add the max_retries
parameter to the signature and use it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well I implemented it as a second method to reduce nesting. I reverted it now, please have a look at it and let me know which version you like more: (a) all in one method or (b) split up into two methods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one last thing 🥇
Co-authored-by: Jörg Zimmermann <[email protected]>
No description provided.