run.pyのgenerate_appなどの関数を分離していく

[Hiroshiba]

内容

run.pyが複雑化し、テストもない状態でメンテナンスが難しくなりつつあります。

例

• cors_policy_modeとallow_originをgenerate_appの引数にする #500

メンテナンス性を挙げるためにも、run.pyにあるいろんな関数を分離させて解体していくのがこのissueの目的です。

Pros 良くなる点

Cons 悪くなる点

その他

関連

• upload-docs の CI が落ちるようになった #499
• cors_policy_modeとallow_originをgenerate_appの引数にする #500
• run.pyの改善 #119

[tarepan]

run.pyにあるいろんな関数を分離

@Hiroshiba
こちらの意図は

• run.py 以外のファイル/モジュールへ分離
• run.py の巨大 generate_app() 関数を更に小さな関数へ分解する（あくまで run.py 内に留める）

のどちらでしょうか？

[Hiroshiba]

@tarepan たしかにどちらかわからないですね…！

「メンテナンス生を上げるため」とあるので、読みやすくリファクタリングされていればどちらでも良さそうです。
ただまぁ関数が多いので、ファイル分けするのが妥当なのかなと感じます！

[tarepan]

2024年3月現在の run.py を調査した。
その結果、これまでのリファクタリングによりほぼ分割・切り出しが完了していることが確認された。
積極的に切り出しする意味があるのは FIXME がついた以下の1箇所のみ：

voicevox_engine/run.py

Lines 877 to 878 in 1968353

	# FIXME: この関数をどこかに切り出す
	def _speaker_info(

現状で別ファイル/モジュールに移植できるのは 30 行程度の set_output_log_utf8() 関数のみであった。
これだけのためにモジュールを用意するのは過剰であると考える。

@Hiroshiba
本 issue は役割を果たしたと考えます（close 可能？）

[Hiroshiba]

あ、肝心のgenerate_appの切り出しがまだかもです！
とはいえ切り出しの要不要は要議論かもですね…。
run.pyは1500行くらいあるので、なにかしらの切り出しちゃったほうが良いと思ってます。

これはこのissueの範疇じゃなさそうですが、1番やった方が良さそうなのはgenerate_app自体の切り崩しかもです。
辞書や音声合成やプリセットなどの概念にAPI関数を分けていく、みたいな。

タスクを整理して別のissueでまとめ直すか、generate_appがしかるべき場所（どこ…？）に移動すればこのissueはcloseで良いと思います！

[tarepan]

generate_appの切り出し

私の理解が合っていないような気がするので、認識合わせさせてください。

generate_app() は 1250 行の巨大関数ですが、そのほぼ全行が app への API 追加 (例: @app.post()) です。
generate_app() を分割する場合、app 変数を引き回し、サブ関数内で @app.post() により API 追加する、という方向性で認識合っているでしょうか？

[Hiroshiba]

どういう構成が最適かは僕もわかってないのですが、app渡すのはテストが少し大変かもですね。
FastAPIの公式ドキュメント読むに、appを渡すのではなくrouterを返してもらう形にすると上手く書けるかも…？
https://fastapi.tiangolo.com/ja/tutorial/bigger-applications/

[tarepan]

着手しています。