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

feat: Refactor OpenAIClient initialization and add client parameter #140

Merged
merged 1 commit into from
Sep 21, 2024

Conversation

danielchalef
Copy link
Member

@danielchalef danielchalef commented Sep 21, 2024

The code changes refactor the OpenAIClient initialization to accept an optional client parameter. This allows the client to be passed in from outside, providing more flexibility and enabling easier testing.


Important

Refactor OpenAIClient to accept an optional client parameter for increased flexibility and testability.

  • Initialization:
    • OpenAIClient.__init__ now accepts an optional client parameter.
    • If client is not provided, defaults to creating a new AsyncOpenAI instance.
  • Attributes:
    • client attribute can now be externally provided, enhancing flexibility and testability.

This description was created by Ellipsis for af90401. It will automatically update as commits are pushed.

The code changes refactor the `OpenAIClient` initialization to accept an optional `client` parameter. This allows the client to be passed in from outside, providing more flexibility and enabling easier testing.
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! Reviewed everything up to af90401 in 8 seconds

More details
  • Looked at 57 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/openai_client.py:60
  • Draft comment:
    Consider using a more specific type for the client parameter instead of typing.Any for better type checking and readability.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The client parameter is typed as typing.Any, which is not specific. It would be better to use a more specific type if possible to improve type checking and code readability.
2. graphiti_core/llm_client/openai_client.py:59
  • Draft comment:
    The __init__ method is handling multiple responsibilities. Consider refactoring to adhere to the Single Responsibility Principle, such as separating client initialization into a dedicated method.
  • Reason this comment was not posted:
    Confidence changes required: 70%
    The constructor method is handling multiple responsibilities, including configuration setup and client initialization.

Workflow ID: wflow_u2XJbPwybBwSbCws


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

@danielchalef danielchalef merged commit 9b71b46 into main Sep 21, 2024
6 checks passed
@danielchalef danielchalef deleted the feat/pass-in-openai-client branch September 21, 2024 19:09
@github-actions github-actions bot locked and limited conversation to collaborators Sep 21, 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