実行環境とサーバーのリファクタリング #52
Conversation
Summary of ChangesHello @bobtaroh, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! このプルリクエストは、開発環境のセットアッププロセスとサーバーサイドのAPI構造を大幅にリファクタリングすることを目的としています。特に、アプリケーションのセットアップスクリプトをより明確に分離し、VS Codeの設定を改善して開発体験を向上させました。また、サーバーサイドでは、以前のユーザー管理中心のAPIから、旅行プラン、ページ、ブロックという新しいエンティティを中心としたAPIに移行し、関連するCRUD操作、Pydanticスキーマ、およびテストを全面的に再構築しました。これにより、将来の機能拡張が容易になり、コードベースの保守性が向上します。 Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
このプルリクエストは、開発環境のセットアップスクリリプト、VSCode設定、npmスクリプトをリファクタリングし、サーバーサイドのAPIを大幅に再構築するものです。特に、Trip, Page, Blockという新しいモデルに対応するCRUD操作とAPIエンドポイントが追加されています。全体として、開発体験を向上させ、APIの構造を整理するための良い変更です。ただし、1点クリティカルな問題点を指摘しました。新しく追加されたサーバーサイドのコードにおいて、非同期関数(async def)内で同期的なデータベース操作が行われており、これはパフォーマンス上の深刻な問題を引き起こす可能性があります。この問題は複数のファイルにまたがるため、修正方法の詳細は各コメントに記載しましたので、ご確認をお願いします。
| async def create_block(db: Session, block: BlockCreate, page_id: int) -> Block: | ||
| """ | ||
| ブロックをデータベースに作成する関数 | ||
|
|
||
| Args: | ||
| db (Session): データベースセッション | ||
| block (BlockCreate): ブロック作成用のスキーマ | ||
| page_id (int): ページのID | ||
|
|
||
| Returns: | ||
| Block: 作成されたブロック | ||
| """ | ||
| db_block = Block(**block.model_dump(), page_id=page_id) | ||
| db.add(db_block) | ||
| db.commit() | ||
| db.refresh(db_block) | ||
| return db_block |
There was a problem hiding this comment.
非同期関数内での同期I/Oブロッキング
現在の実装では、CRUD関数(create_block, find_blocksなど)がasync defで定義されていますが、内部で使用されているSQLAlchemyのSession (db.add, db.commit, db.queryなど)は同期的なブロッキングI/Oです。
FastAPIのasyncエンドポイント内でこれらの同期関数を呼び出すと、イベントループがブロックされ、アプリケーション全体のパフォーマンスが著しく低下する原因となります。これはpages.pyやtrips.pyのCRUD関数にも共通する問題です。
この問題を解決するには、データベースアクセスが同期的であるため、関連する関数をすべて同期的にする必要があります。
推奨される修正方法は以下の通りです。
server/app/cruds/ディレクトリ内のすべてのCRUD関数 (blocks.py,pages.py,trips.py) をasync defからdefに変更してください。server/app/routers/ディレクトリ内の、これらのCRUD関数を呼び出しているエンドポイント関数もasync defからdefに変更し、CRUD関数呼び出し時のawaitを削除してください。FastAPIは同期的なパスオペレーション関数をスレッドプールで実行するため、イベントループはブロックされません。server/tests/ディレクトリ内の関連するテストやfixtureも同様にasyncから同期的な実装に修正してください(@pytest.mark.asyncioやasync defの削除、awaitの削除など)。
以下にcreate_block関数の修正例を示します。この修正をこのファイル内のすべての関数、および関連する他のファイルにも適用してください。
| async def create_block(db: Session, block: BlockCreate, page_id: int) -> Block: | |
| """ | |
| ブロックをデータベースに作成する関数 | |
| Args: | |
| db (Session): データベースセッション | |
| block (BlockCreate): ブロック作成用のスキーマ | |
| page_id (int): ページのID | |
| Returns: | |
| Block: 作成されたブロック | |
| """ | |
| db_block = Block(**block.model_dump(), page_id=page_id) | |
| db.add(db_block) | |
| db.commit() | |
| db.refresh(db_block) | |
| return db_block | |
| def create_block(db: Session, block: BlockCreate, page_id: int) -> Block: | |
| """ | |
| ブロックをデータベースに作成する関数 | |
| Args: | |
| db (Session): データベースセッション | |
| block (BlockCreate): ブロック作成用のスキーマ | |
| page_id (int): ページのID | |
| Returns: | |
| Block: 作成されたブロック | |
| """ | |
| db_block = Block(**block.model_dump(), page_id=page_id) | |
| db.add(db_block) | |
| db.commit() | |
| db.refresh(db_block) | |
| return db_block |
| async def create_block( | ||
| page_id: int, block: BlockCreate, db: Session = Depends(get_db_session) | ||
| ) -> Block: |
There was a problem hiding this comment.
非同期関数内での同期I/Oブロッキング
cruds/blocks.pyへのコメントで指摘したように、同期的なDB操作を行うため、このエンドポイントも同期関数(def)として定義する必要があります。FastAPIは同期関数をスレッドプールで実行するため、イベントループをブロックしません。
この修正は、routersディレクトリ内の他のエンドポイント(pages.py, trips.py)にも適用してください。
| async def create_block( | |
| page_id: int, block: BlockCreate, db: Session = Depends(get_db_session) | |
| ) -> Block: | |
| def create_block( | |
| page_id: int, block: BlockCreate, db: Session = Depends(get_db_session) | |
| ) -> Block: |
bobtaroh
force-pushed
the
feature/issue51_refactor-server-test-and-env
branch
from
b35cbc9 to
f020d94
Compare
bobtaroh
changed the base branch from
develop
to
feature/issue38_add-server-test-system
|
Visit the preview URL for this PR (updated for commit 9bedd90): https://tabi-share-8ef6b--pr52-feature-issue51-refa-9lj77khl.web.app (expires Wed, 31 Dec 2025 14:51:49 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 9f2a87ede127df7673322845e34cf22c1372d720 |
|
mypyが型の違いを許してくれるように設定や実装にできないか、調査しよう。 |
1 participant
概要
詳細
動作確認
フロントに変更がある場合、画像を添付すること
確認項目