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

Fix and improve retry mechanism for Compass client #84

Merged
merged 2 commits into from
Feb 5, 2025

Conversation

corafid
Copy link
Member

@corafid corafid commented Feb 5, 2025

Some APIs, e.g. search APIs, were hard-coding the retry count and sleep time to 1 and 1, respectively. Others, used the DEFAULT_MAX_RETRIES and DEFAULT_SLEEP_RETRY_SECONDS constants, respectively. Yet others allowed passing values from the clients. This PR fixes this as follows:

  1. Introduce default_max_retries and default_sleep_retry_seconds at the client level, which gets applied to all APIs by default.
  2. Each API optionally has max_retries and sleep_retry_seconds parameters that, if specified, will override the default values from the client.

@corafid corafid requested a review from a team as a code owner February 5, 2025 02:34
@corafid corafid force-pushed the corafid/fix/retry-mechanism branch from b5d23ce to 9cd32f2 Compare February 5, 2025 02:38
luca-cohere
luca-cohere previously approved these changes Feb 5, 2025
Copy link
Contributor

@luca-cohere luca-cohere left a comment

Choose a reason for hiding this comment

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

I left a comment about an improvement, definitely not a blocker but constraining seconds and retries to be > 0 or None can be useful.

Some APIs, e.g. search APIs, were hard-coding the retry count and sleep
time to 1 and 1, respectively. Others, used the DEFAULT_MAX_RETRIES and
DEFAULT_SLEEP_RETRY_SECONDS constants, respectively. Yet others allowed
passing values from the clients. This PR fixes this as follows:

1. Introduce `default_max_retries` and `default_sleep_retry_seconds` at
   the client level, which gets applied to all APIs by default.
2. Each API optionally has `max_retries` and `sleep_retry_seconds`
   parameters that, if specified, will override the default values from
   the client.
@corafid corafid force-pushed the corafid/fix/retry-mechanism branch from 5fc8622 to 31457ba Compare February 5, 2025 16:25
@corafid corafid merged commit 56d8e03 into main Feb 5, 2025
11 checks passed
@corafid corafid deleted the corafid/fix/retry-mechanism branch February 5, 2025 18:37
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.

3 participants