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

Docs: 添加对 fastapi_reload 在 Windows 平台额外影响的说明 #830

Merged
merged 14 commits into from
Feb 22, 2022

Conversation

CherryGS
Copy link
Contributor

@CherryGS CherryGS commented Feb 20, 2022

在 python 3.8 之后 , win平台默认使用 ProactorEventLoop , 而 uvicorn 在启动 reload 后会强制绑定默认事件循环为 SelectorEventLoop , 后者在 win 平台的可使用性不如前者 , 也会导致一些使用 asyncio 的包出错(如 playwright)
以下是一些相关信息
uvicorn#529
python doc

@codecov
Copy link

codecov bot commented Feb 20, 2022

Codecov Report

Merging #830 (2632854) into master (a8a6eb8) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #830   +/-   ##
=======================================
  Coverage   76.41%   76.41%           
=======================================
  Files          42       42           
  Lines        3083     3083           
=======================================
  Hits         2356     2356           
  Misses        727      727           
Flag Coverage Δ
unittests 76.41% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a8a6eb8...2632854. Read the comment docs.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 20, 2022

🚀 Deployed on https://deploy-preview-830--nonebot2.netlify.app

@github-actions github-actions bot temporarily deployed to pull request February 20, 2022 14:55 Inactive
@mnixry
Copy link
Member

mnixry commented Feb 20, 2022

根据 @SK-415 反馈, 似乎beta2锁定的uvicorn版本^0.17.0 又强制换回了SelectorEventLoop, 然后彻底禁用了Windows平台的重载, 你可以检查一下是不是这样

@CherryGS
Copy link
Contributor Author

环境
捕获
0

代码
2
1

不开启 reload
3

开启 reload
4

开启 reload 后启动会有 warning
5

@SK-415
Copy link
Contributor

SK-415 commented Feb 20, 2022

你说的情况在低版本 uvicorn<=1.14.0 中确实存在,Windows 系统下 ProactorEventLoop 会被替换为 SelectorEventLoop,进而导致需要调用 asyncio.subprocess.create_subprocess_* 的库,如 Playwright 无法正常工作。我也曾写过一个 补丁 强行替换回默认事件循环以解决这个问题。

但是在 encode/uvicorn#1070 中,为了适配 Python 3.1.0,Windows 下原本用于替换 ProactorEventLoop 的事件循环从 SelectorEventLoop 变为了 WindowsSelectorEventLoopPolicy。经过实测,Playwright 和 --reload 模式均可在 WindowsSelectorEventLoopPolicy 下正常工作,无需做任何修改。

然后在 encode/uvicorn#1257 中,Windows 系统带了 --reload 参数的情况下才会被替换默认事件循环,在此之前并不会区分,而是统一根据平台修改。上文截图中的 warning 也是在这次更新中添加的。

上述改动在 uvicorn>=1.16.0 之后均有效,因此只要保证 Uvicorn 为较新版本就不存在 encode/uvicorn#529 的问题。

@CherryGS
Copy link
Contributor Author

CherryGS commented Feb 20, 2022

0
在这个版本下 , 添加 reload 行为启动后 playwright 仍然会报错 , 这也是 pypi 能获取到的最高版本了

在 env 里定义 fastapi_reload=true , 就会启动 reload , 同时有一条 warning , 见上

@SK-415
Copy link
Contributor

SK-415 commented Feb 20, 2022

确认,问题确实依然存在,感谢指正。

我没有注意到 Nonebot2 增加了 fastapi_reload 配置且默认为 False,导致我误以为启动了 reload 模式,实则根据 encode/uvicorn#1257 使用的依然是默认事件循环。

改正后重新测试,确认使用的是 SelectorEventLoop,Playwright 等库依然无法使用。

@yanyongyu yanyongyu added the documentation Improvements or additions to documentation label Feb 20, 2022
@SK-415
Copy link
Contributor

SK-415 commented Feb 20, 2022

但是我觉得这条提示的信息量有点少,对于没有经验的开发者很难意识到 “事件循环强制绑定到 SelectorEventLoop” 会有什么影响。

我的建议是找个地方(比如就这里)总结一下可能产生的问题,并在提示最后加一句类似于:“可能导致部分库无法正常运行(详见……)” 并附上总结的链接。

@github-actions github-actions bot temporarily deployed to pull request February 21, 2022 12:11 Inactive
@github-actions github-actions bot temporarily deployed to pull request February 21, 2022 14:34 Inactive
mnixry
mnixry previously approved these changes Feb 21, 2022
Copy link
Member

@mnixry mnixry left a comment

Choose a reason for hiding this comment

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

感谢PR, 可以稍微改一下上述这些地方的表述

另外请将修改后的文档同步到website/docs/tutorial, 以便使未来版本的文档也包含该更改

CherryGS and others added 6 commits February 21, 2022 22:43
…iver.md

Co-authored-by: Mix <32300164+mnixry@users.noreply.github.com>
…iver.md

Co-authored-by: Mix <32300164+mnixry@users.noreply.github.com>
…iver.md

Co-authored-by: Mix <32300164+mnixry@users.noreply.github.com>
…iver.md

Co-authored-by: Mix <32300164+mnixry@users.noreply.github.com>
…iver.md

Co-authored-by: Mix <32300164+mnixry@users.noreply.github.com>
@mnixry mnixry changed the title 添加 fastapi_reload 在 win 的额外影响 Docs: 添加对 fastapi_reload 在 Windows 平台额外影响的说明 Feb 21, 2022
@github-actions github-actions bot temporarily deployed to pull request February 21, 2022 14:51 Inactive
@github-actions github-actions bot temporarily deployed to pull request February 21, 2022 14:54 Inactive
@github-actions github-actions bot temporarily deployed to pull request February 22, 2022 11:02 Inactive
Copy link
Member

@mnixry mnixry left a comment

Choose a reason for hiding this comment

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

感谢PR,将会在稍后合并。

@mnixry mnixry merged commit 2c271da into nonebot:master Feb 22, 2022
@CherryGS CherryGS deleted the patch-1 branch February 22, 2022 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Development

Successfully merging this pull request may close these issues.

4 participants