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: FastAPI optional 依存パッケージの集約 #1077

Closed
tarepan opened this issue Feb 26, 2024 · 12 comments
Closed

add: FastAPI optional 依存パッケージの集約 #1077

tarepan opened this issue Feb 26, 2024 · 12 comments

Comments

@tarepan
Copy link
Contributor

tarepan commented Feb 26, 2024

VOICEVOX ENGINE と FastAPI optional 依存パッケージの関係性に関する記録

追記: fastapi[all] による集約を検討中
追記2: 依存 issue #995

パッケージと関係性

FastAPI は以下の optional dependencies を持つ。
VOICEVOX ENGINE はその一部のみへ、環境に応じて依存している。

パッケージ名 依存
httpx test
jinja2 prod
python-multipart prod
itsdangerous -
pyyaml prod
ujson -
orjson -
email_validator -
uvicorn[standard] prod
pydantic-settings -
pydantic-extra-types -

fastapi[all] の可否

fastapi[all] により、パッケージを pyproject.toml へ記述し明示的に管理せずとも導入できる。
しかし上記からわかるように、production 環境であれば 4/11 の導入のみで済む。
製品サイズやバグリスクを低減する観点から、個別パッケージの明示的管理には一定のメリットがある。

関連 issues

@Hiroshiba
Copy link
Member

allで良い気もします!
なんかでかいファイルが知らない間に追加されないか若干不安ですが、まあ大丈夫かなと!

@tarepan
Copy link
Contributor Author

tarepan commented Feb 26, 2024

allで良い

👍
パッケージ管理の簡略化優先という方針ですね。

依存 issues

下記 issues / PRs が解決し次第、着手可能:

@sabonerune
Copy link
Contributor

表だとuvicorn[standard]prodになっていますけれどstandardは入っていません。
その辺りの依存関係も入ってきますね。

https://www.uvicorn.org/#quickstart
大きな影響を与えそうなのはuvloop?
asyncioのイベントループを置き換えて高速化するがWindows非対応。

@Hiroshiba
Copy link
Member

Hiroshiba commented Feb 26, 2024

@sabonerune なるほどです!
uvloopが入ると Windows は実行に失敗するとかそういう感じでしょうか・・・?


まあでも変更後1回ビルドしてみて動くのかどうかは確かめても良さそうに思いました。
ビルドして確かめるのは方法が2つあって、1つがこのリポジトリに取り込む前にworkflow_dispatchでGithub Actionsでビルドする方法(forkリポジトリでも可能)、あとはmasterにマージした後の自動dev releaseビルドを確認する方法と。

workflow_dispatchでビルドする方法がそういえば案内できてなかったので追記しようと思います。

@sabonerune
Copy link
Contributor

Uvloopが使用可能な場合は自動的に使用するというようになっていたので失敗することはなさそうです。

Windowsとそれ以外で細かい挙動が変わるかも?くらいでしょうか。

@Hiroshiba
Copy link
Member

なるほどです!!
動かないとかになったら厄介ですが、まあ大丈夫な気がしますね…!たぶん!

@tarepan tarepan changed the title 記録: VOICEVOX ENGINE と FastAPI optional 依存パッケージ add: FastAPI optional 依存パッケージの集約 Mar 1, 2024
@tarepan tarepan added the 状態:実装者募集 実装者を募集している状態 label Mar 5, 2024
@sabonerune
Copy link
Contributor

sabonerune commented Mar 6, 2024

試しにpoetry add fastapi[all]を実行したところいくつか問題が発生したのでメモしておきます。

pydantic-settingspydantic-extra-typespydantic>=2.0が必要です。
大きな変更が入っているため恐らく修正が必要です
https://docs.pydantic.dev/latest/migration/
ref: #995

uvicorn[standard]=^0.15.0が依存するhttptools==0.2.*にはpy311のwheelがリリースされていません。また自分の環境(Windows)ではビルドが通りませんでした。
恐らくuvicornを0.17.6以上にする必要があります。(多分もっと上げても平気)


その他

pydanticpyproject.tomlに入っていないことに気づきました。
fastapiにほぼ間違いなく含まれているとはいえVOICEVOXのコード内で直接使われているパッケージは依存に含めるべきかもしれない?
(starletteも?)

@tarepan
Copy link
Contributor Author

tarepan commented Mar 6, 2024

@sabonerune
検証ありがとうございます!

pydantic>=2.0が必要 ... 恐らく修正が必要

👍
影響範囲が ENGINE の隅々までに渡るので、本 issue 実装前の pydantic 2.0 移行は MUST ですね。クリティカルな情報です、ありがとうございます。

pydanticpyproject.tomlに入っていない ... starlette

👍
直接依存は ENGINE 側で管理すべきですね、バグです。
issue open 及び PR 頂ければ早急に @tarepan が対応します。

@tarepan tarepan removed the 状態:実装者募集 実装者を募集している状態 label Mar 6, 2024
@tarepan tarepan self-assigned this Mar 6, 2024
@sabonerune
Copy link
Contributor

とりあえず0.17.0ブランチにdependencies

uvicorn[standard]
httpx
jinja2
python-multipart
itsdangerous
ujson
orjson
email-validator

を入れる形である程度fastapi[all]を再現してビルドしてみました。

とりあえずgithub workflowのテストは通ったので動作に問題が起こる可能性は低いと思います。


次にファイルサイズですが次のように変化しました

圧縮前 7z圧縮後
Windows-cpu 1308MB -> 1311MB(+3MB) 1118MB -> 1120MB(+2MB)
macOS 1370MB -> 1375MB(+5MB) 1123MB -> 1125MB(+2MB)
Linux-cpu 1396MB -> 1415MB(+19MB) 1135MB -> 1140MB(+5MB)

増加量がWindows < macOS < Linuxとなっているのは恐らくuvloopが原因ですね。
(Windows非対応・LinuxのバイナリサイズがMacより大きい)


そこまで大したサイズではない感じ?
pyinstallerのオプション弄ってパッケージを除外する手も使えますがかえって管理コストが上がる?

@tarepan
Copy link
Contributor Author

tarepan commented Mar 8, 2024

ある程度fastapi[all]を再現してビルド ... 1415MB(+19MB) ... そこまで大したサイズではない感じ?

👍
丁寧な検証ありがとうございます!
最大で圧縮前 1.3% 増なら、私も許容範囲に感じます。

pyinstallerのオプション弄ってパッケージを除外 ... かえって管理コストが上がる?

👍
同意です。そこをいじるなら「fastapi + 明示的必要パッケージ」の方が軽いしわかりやすそうです。

@Hiroshiba
Copy link
Member

まあ導入してもしなくてもどちらでも良さそうかなと思いました!
導入しちゃった方が全体的に品質が上がるなら導入しちゃって良いと思います。
今は @tarepan さんのやりやすい方で良いのかなと思います!

@tarepan
Copy link
Contributor Author

tarepan commented Mar 19, 2024

  • メリット
    • 3 パッケージを pyproject.yaml で明示的管理しなくてよくなる
  • デメリット
    • ストレージ 1.3 %増
    • 暗示的パッケージ管理由来の配慮コスト(例: httptools
  • コスト
    • pydantic メジャー bump
    • uvloop 周り影響評価

メリットに比べデメリットとコストが重いため、現時点では NoGo とします。
検証にご協力下さった皆さん、ありがとうございました!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants