-
Notifications
You must be signed in to change notification settings - Fork 514
fix: set WindowsSelectorEventLoopPolicy only for curl-impersonate template without playwright
#1209
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: set WindowsSelectorEventLoopPolicy only for curl-impersonate template without playwright
#1209
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR scopes the WindowsSelectorEventLoopPolicy setup to only curl-impersonate projects that don’t include Playwright, preventing ProactorEventLoop issues on Windows while keeping default behavior elsewhere.
- Wraps the
import platformand event loop policy call in a cookiecutter condition for curl-impersonate without Playwright - Removes the unconditional policy application and old guidance comment
- Inserts blank-line helpers around conditional blocks
| @@ -1,12 +1,15 @@ | |||
| import asyncio | |||
| # % if cookiecutter.http_client == 'curl-impersonate' and 'playwright' not in cookiecutter.crawler_type | |||
Copilot
AI
May 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using # % to denote Jinja directives can be confusing—consider using {% if ... %}/{% endif %} so the templating syntax is more explicit and easier to maintain.
| import platform | ||
|
|
||
| # % endif | ||
| {{ '' }} |
Copilot
AI
May 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The {{ '' }} blank-line insertion may be unnecessary and could be replaced with a simpler newline in the template to improve readability.
| {{ '' }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using an empty string, ignored when creating a result
| if platform.system() == 'Windows': | ||
| # This mitigates a warning raised by curl-cffi. If you do not need to use curl-impersonate, you may remove this. | ||
| # This mitigates a warning raised by curl-cffi. |
Copilot
AI
May 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The original guidance explaining how to remove this policy if not using curl-impersonate was removed—consider re-adding a brief note for users who may not need this workaround.
|
|
||
|
|
||
| if __name__ == '__main__': | ||
| # % if cookiecutter.http_client == 'curl-impersonate' and 'playwright' not in cookiecutter.crawler_type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should print a warning in case playwright+curl-cffi is selected, something like "Hey, curl-cffi will complain about the event loop policy, but that won't work with playwright, so just ignore the warning"
|
Did you have a chance to test this on Windows @Mantisus? |
Yep 😊 |
vdusek
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
WindowsSelectorEventLoopPolicyonly for templates that use curl-impersonate, provided that playwright is not used. Since curl-impersonate does not supportProactorEventLoopused in Windows.For all other cases,
ProactorEventLoopis recommended for Windows.Issues