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

Update event loop on windows only for old curl_cffi #1833

Merged
merged 1 commit into from
Apr 17, 2024

Conversation

hlohaus
Copy link
Collaborator

@hlohaus hlohaus commented Apr 13, 2024

No description provided.

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 "Update event loop on windows only for old curl_cffi":

Pull Request Review

Summary

The pull request introduces a conditional check for the Windows event loop policy to ensure better compatibility with older versions of curl_cffi that do not have the _get_selector attribute. Additionally, it modifies the streaming callback in provider.py to prevent stopping on user tokens.

Code Review

  • The addition of the should_not_stop function in provider.py is a clever way to filter out system tokens during streaming. This should maintain the flow of user-generated tokens without interruption.
  • In base_provider.py, the use of a try-except block to import curl_cffi and check for the _get_selector attribute is a good defensive programming practice. It ensures that the new event loop policy is set only when necessary, which could prevent potential issues with newer versions of curl_cffi.
  • The code is well-structured and follows Python's PEP 8 style guide, which is great for maintainability.

Suggestions

  • It would be beneficial to include a brief description in the pull request to explain the context and the reason for these changes. This helps reviewers and future maintainers understand the purpose of the modifications.
  • Consider adding a comment above the should_not_stop function to explain its role in the streaming process for clarity.

Overall, the changes are sound and appear to address a specific compatibility issue with curl_cffi. Great work!


I hope this review is helpful. If you have any questions or need further assistance, feel free to reach out.

Best regards,
g4f Copilot

g4f/locals/provider.py Show resolved Hide resolved
g4f/locals/provider.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