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

Implement retry for LLMClient #44

Merged
merged 4 commits into from
Aug 26, 2024
Merged

Implement retry for LLMClient #44

merged 4 commits into from
Aug 26, 2024

Conversation

danielchalef
Copy link
Member

@danielchalef danielchalef commented Aug 26, 2024

This pull request implements retry functionality for the LLMClient class in the client.py file. The _generate_response_with_retry method has been added, which retries the generation of a response in case of a server error. The tenacity library is used for the retry logic. This enhancement improves the reliability of the client by handling temporary server errors gracefully.


🚀 This description was created by Ellipsis for commit b32d46f

Summary:

This PR adds retry functionality to LLMClient using tenacity for handling server errors and updates dependencies.

Key points:

  • Adds retry functionality to LLMClient in graphiti_core/llm_client/client.py.
  • Uses tenacity for retry logic with exponential backoff.
  • Introduces _generate_response_with_retry method for handling server errors.
  • Updates generate_response to use retry logic.
  • Catches and logs server and unexpected errors.
  • Updates pyproject.toml to include tenacity.

Generated with ❤️ by ellipsis.dev

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to 52f5f10 in 13 seconds

More details
  • Looked at 69 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. graphiti_core/llm_client/client.py:65
  • Draft comment:
    The _is_server_error method should be used to check an instance of an exception, not the exception class itself. Pass the exception instance to _is_server_error.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_QmpODbYjon2Lxyrs


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

graphiti_core/llm_client/client.py Outdated Show resolved Hide resolved
Copy link
Contributor

ellipsis-dev bot commented Aug 26, 2024

Skipped PR review on ced3fab because no changed files had a supported extension. If you think this was in error, please contact us and we'll fix it right away.


Generated with ❤️ by ellipsis.dev

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on b32d46f in 25 seconds

More details
  • Looked at 21 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. graphiti_core/llm_client/client.py:91
  • Draft comment:
    Consider expanding the retry logic to handle other transient errors, such as network issues, not just server errors. This can improve the robustness of the retry mechanism.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The retry logic is implemented using the tenacity library, which is a good choice for handling retries. However, the retry logic is only applied to server errors (HTTP 5xx). The code should also handle other types of transient errors, such as network issues, which are not currently covered by the is_server_error function.
2. graphiti_core/llm_client/client.py:91
  • Draft comment:
    Consider adding comments to explain the retry logic in _generate_response_with_retry for better clarity.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code is mostly well-structured, but there are a few areas that could be improved for clarity and maintainability.

Workflow ID: wflow_AcCVCLAmye20HY1I


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@danielchalef danielchalef merged commit fc4bf3b into main Aug 26, 2024
6 checks passed
@danielchalef danielchalef deleted the feat/llm-retry branch August 26, 2024 19:53
@github-actions github-actions bot locked and limited conversation to collaborators Aug 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants