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

Add authless OpenaiChat #1789

Merged
merged 1 commit into from
Apr 5, 2024
Merged

Add authless OpenaiChat #1789

merged 1 commit into from
Apr 5, 2024

Conversation

hlohaus
Copy link
Collaborator

@hlohaus hlohaus commented Apr 5, 2024

No description provided.

@hlohaus hlohaus merged commit c791012 into xtekky:main Apr 5, 2024
1 check passed
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hello H Lohaus,

Thank you for your contribution to the project. Here's the review for your pull request titled "Add authless OpenaiChat":

General Feedback

It's great to see efforts towards making OpenaiChat accessible without authentication. This could potentially simplify the process for users to interact with the chat service.

Code Review

  • The removal of the needs_auth flag and the addition of the action parameter in the create method are significant changes. It's important to ensure that these changes do not affect the existing functionality and security.
  • The introduction of the NoValidHarFileError exception is a good practice, as it provides a clearer error message when no .har file is found.
  • The refactoring of the get_default_model method to handle anonymous access is a thoughtful addition. However, the handling of 401 status codes by raising a MissingAuthError should be carefully documented for future maintainers.
  • The changes in the create_async_generator method seem to accommodate the new authless feature. However, the logic appears complex and might benefit from further comments or documentation for clarity.
  • The addition of get_default_headers as a static method helps centralize header management, which is a good practice for maintainability.

Suggestions

  • Ensure thorough testing, especially for the new authless feature, to prevent any security loopholes.
  • Consider adding more comments to complex sections of the code to aid in maintainability.
  • Update the documentation to reflect the new changes and guide users on how to use the authless feature.

Overall, the pull request introduces some promising features. With proper testing and documentation, it could be a valuable addition to the project.

Best regards,
g4f Copilot

g4f/Provider/needs_auth/OpenaiChat.py Show resolved Hide resolved
g4f/Provider/needs_auth/OpenaiChat.py Show resolved Hide resolved
cls._update_request_args(session)
if response.status == 401:
raise MissingAuthError('Add a "api_key" or a .har file' if cls._api_key is None else "Invalid api key")
await raise_for_status(response)
data = await response.json()
if "categories" in data:
cls.default_model = data["categories"][-1]["default_model"]
return cls.default_model
Copy link

Choose a reason for hiding this comment

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

Directly passing data to ResponseError might expose sensitive information. Consider sanitizing the data or using a more generic error message.

g4f/Provider/needs_auth/OpenaiChat.py Show resolved Hide resolved
g4f/Provider/needs_auth/OpenaiChat.py Show resolved Hide resolved
g4f/errors.py Show resolved Hide resolved
g4f/errors.py Show resolved Hide resolved
g4f/models.py Show resolved Hide resolved
g4f/models.py Show resolved Hide resolved
g4f/models.py Show resolved Hide resolved
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.

1 participant