-
Notifications
You must be signed in to change notification settings - Fork 2
update #51
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
Conversation
Walkthroughドキュメント(CLAUDE.md, README.md)の更新。アーキテクチャ説明を粒度の細かいモジュール単位に再編し、環境判定とDIモジュールの記述を明確化。複数UIフレームワークやテスト構成の詳細を追記。READMEは composeファイル名・手順表記を更新し、Azureデプロイ手順を拡充。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as User
participant App as Application
participant Settings as RunEnv.judge_from_env()
participant DI as DI Module Selector
participant Repo as MessageRepository
User->>App: 起動/リクエスト
App->>Settings: 環境変数を評価
Settings-->>App: 実行環境(LOCAL/TEST/GCP/AWS/AZURE)
App->>DI: 環境に対応するモジュールを要求
DI-->>App: 具体実装(SQLite/Firestore/DynamoDB/CosmosDB)
App->>Repo: メッセージ登録/履歴取得
Repo-->>App: 結果
App-->>User: 応答
note over Settings,DI: 新/明確化された環境検出とDI割当の流れ
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
要約このPRは、Pythonウェブアプリケーションテンプレートのドキュメントを改善し、オンionアーキテクチャや依存性注入、クラウドデプロイメントに関する情報を追加しています。これにより、開発者がプロジェクトをより理解しやすくなります。 変更点
Uplevel your code reviews with CodeRabbit ProCodeRabbit ProIf you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects. |
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.
Files selected (2)
- CLAUDE.md (3)
- README.md (2)
Files skipped from review due to trivial changes (1)
- README.md
Review comments generated (3)
- Review: 0
- LGTM: 3
Tips
Chat with
CodeRabbit Bot (@coderabbitai)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitaiin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨No code suggestions found for the PR. |
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CLAUDE.md (1)
94-102: .gitignore に credentials/ を追加して Git 管理から除外する検証結果: リポジトリルートの .gitignore に credentials/ の除外ルールは見つかりませんでした。
対応:
- ファイル: .gitignore(リポジトリルート)に次を追加: credentials/
- 既に credentials/ 内の機密がコミット済みの場合は履歴から削除(git filter-repo または BFG を使用)し、該当キー/シークレットをローテーションすること。
🧹 Nitpick comments (7)
README.md (4)
88-90: compose.yml とパス表記の具体性を上げて誤解を防止credentials のマウント削除手順は対象サービス名/volumes キーが不明です。例示を入れてください(対象サービスとマウント先パス)。またファイル/ディレクトリ名はすべてバッククォートで統一しましょう。
例(ドキュメント差分イメージ):
-2. `compose.yml`からcredentialsのマウントを削除 +2. `compose.yml` の `services.app.volumes` から `./credentials:/app/credentials:ro` を削除 + 例: + ```yaml + services: + app: + volumes: +- - ./credentials:/app/credentials:ro + ```Also applies to: 92-92
100-101: ファイル名にバッククォート+補足説明
compose.override.ymlはファイル名として強調してください。あわせて「Dev Container起動中は override が無効化されるわけではなく、既存 compose が生きているため上書きされない」という趣旨が伝わるよう1文補足を。- 終了せずにDev Containerを起動すると、compose.override.ymlの内容が上書きされずデバッグが利用できません。 + 終了せずに Dev Container を起動すると、`compose.override.yml` が読み込まれても + 既存コンテナが残っているため設定が反映されず、デバッグが有効化されません。 + (`make down` で一度停止してから起動してください)また、この近傍の行 99 の bare URL は MD034 に抵触します。
<http://localhost:<LOCAL_PORT>/>のように山括弧で囲むと回避できます。
235-239: デプロイ手順の期待成果物を明記
make deploy-azure実行後に作成される主なリソース(Container Apps、Container Apps Environment、Log Analytics、Cosmos DB など)と、アクセス URL/エンドポイントの確認方法(例:az containerapp show ... | jq .properties.configuration.ingress.fqdn)を1~2行追記すると利用者がつまずきにくくなります。
241-245: bare URL を Markdown リンクに修正(MD034 対応)該当行の bare URL をリンク形式にしてください。
- cf. https://yotiky.hatenablog.com/entry/azure_exceptionsalert + cf. [Azure 例外検知/アラート設定の例](https://yotiky.hatenablog.com/entry/azure_exceptionsalert)(上部の行 233 の Microsoft Learn へのリンクも同様に
[title](url)形式にすると lint 満足度が上がります)CLAUDE.md (3)
39-55: Onion Architecture の詳細化は方針に合致(ディレクトリへの相対リンク追記を提案)層の粒度と所在が明確になり良いです。さらに各ディレクトリへ相対リンクを置くとナビゲーション性が上がります。
-- **Domain Layer:** `app/domain/message/` - Business entities (Message, ID) and repository interfaces (IMessageRepository) +- **Domain Layer:** [`app/domain/message/`](app/domain/message/) - Business entities (Message, ID) and repository interfaces (IMessageRepository) ... -- **Presentation Layer:** `app/presentation/` - Multiple UI frameworks: +- **Presentation Layer:** [`app/presentation/`](app/presentation/) - Multiple UI frameworks:
64-69: 環境判定の変数一覧は有用 — Azure 判定の補助候補を備考に
CONTAINER_APP_REPLICA_NAMEは Container Apps で妥当です。将来的に Azure App Service をサポートするならWEBSITE_SITE_NAME等も備考に追記しておくと拡張が容易です(現時点では必須ではありません)。
84-88: テスト分離の説明は十分 — 実行例の追記を軽く提案
reset_injector()の言及と in-memory SQLite の明示は良いです。pytest 実行例に E2E を含むターゲット指定を1行だけ補うと親切です。```bash make test # or directly with pytest -pytest -sv +pytest -sv tests/unit +pytest -sv tests/e2e</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 5ebe6099a9eaee11a44e24aa3e213dd09576aa4d and 61172f9794a2cd0e3659c5a6f4930b535f114ebd. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `CLAUDE.md` (3 hunks) * `README.md` (2 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧠 Learnings (1)</summary> <details> <summary>📚 Learning: 2025-07-01T12:20:22.295Z</summary>Learnt from: CR
PR: eycjur/python_project_template#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T12:20:22.295Z
Learning: The project must implement Onion Architecture with clear separation between Domain, Infrastructure, Use Case, and Presentation layers**Applied to files:** - `CLAUDE.md` </details> </details><details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>README.md</summary> 99-99: Bare URL used (MD034, no-bare-urls) --- 233-233: Bare URL used (MD034, no-bare-urls) </details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)</summary> * GitHub Check: test (3.13) * GitHub Check: static_analysis (3.13) </details> <details> <summary>🔇 Additional comments (3)</summary><blockquote> <details> <summary>README.md (1)</summary><blockquote> `109-110`: **Dev Containers への表記更新は妥当** VS Code/Dev Containers への表記更新は現行の用語に沿っていて良いです。 </blockquote></details> <details> <summary>CLAUDE.md (2)</summary><blockquote> `26-26`: **make restart の追加は有用** コンテナ再起動をワンコマンド化できて開発体験が向上します。問題ありません。 --- `58-63`: **DIモジュールの環境マッピングは確認済み — 承認** app/di.py に LocalModule(行68)、TestModule(行75)、GCPModule(行82)、AWSModule(行91)、AzureModule(行98)が定義され、app/settings.py に RUN_ENV = RunEnv.judge_from_env()(行38)が存在します。ドキュメントと一致します。自動検証の追加は任意の改善案です。 </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| #### 前準備 | ||
|
|
||
| 1. Entra IDのアプリを登録からサービスプリンシパルを登録し、クライアントシークレットを発行する | ||
| 2. .envのAZURE_CLIENT_ID,AZURE_CLIENT_SECRETに各値を設定 | ||
| 2. `.env`のAZURE_CLIENT_ID,AZURE_CLIENT_SECRETに各値を設定 | ||
| 3. CosmosDBのデータベースとコレクションを作成 | ||
| 4. サービスプリンシパルにCosmosDBへのアクセス権の付与 | ||
| 4. サービスプリンシパルにCosmosDBへのアクセス権の付与 | ||
| ```shell | ||
| az cosmosdb sql role assignment create \ | ||
| --account-name <CosmosDBのアカウント名> \ | ||
| --resource-group <リソースグループ名> \ | ||
| --scope "/" \ | ||
| --principal-id <EntraIDのエンタープライズアプリケーションの該当アプリケーション(サービスプリンシパル)のオブジェクトID> \ | ||
| --role-definition-id <ロールid=00000000-0000-0000-0000-000000000002> | ||
| ``` | ||
| ``` |
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.
💡 Verification agent
🧩 Analysis chain
Azure 事前準備の環境変数が不足の可能性(デプロイ失敗リスク)
.env に AZURE_CLIENT_ID と AZURE_CLIENT_SECRET だけだと、Makefile/Infra(Terraform など)が要求する ARM_/AZURE_* 変数(例: AZURE_TENANT_ID や ARM_SUBSCRIPTION_ID)が不足しデプロイが失敗する可能性があります。ツール連携に必要な正確なキーを README に列挙してください。
以下のスクリプトで Makefile/infra から参照される Azure 関連の環境変数を棚卸しできます。検出結果に基づき README の前提を更新してください。
さらに、--role-definition-id の GUID はロールごとに固定ですが、記載の 00000000-0000-0000-0000-000000000002 が意図したロールかを念のため公式ドキュメントで再確認をお願いします。
🏁 Script executed:
#!/bin/bash
# Azure 変数の参照を探索
rg -n -C2 -g 'Makefile' -g 'infra/**' -e 'AZURE_|ARM_|TENANT|SUBSCRIPTION|CLIENT_ID|CLIENT_SECRET'
# deploy ターゲット確認
rg -n -C2 -g 'Makefile' -e 'deploy-azure|create-infra-azure|azure'Length of output: 875
🌐 Web query:
Azure Cosmos DB の「Built-in Data Contributor」等の roleDefinitionId が正しいか確認
💡 Result:
はい。Azure Cosmos DB のデータプレーン組み込みロールの roleDefinitionId はドキュメントで固定の値が示されています。主な参照は次のとおりです。
- Cosmos DB Built-in Data Reader — ID: 00000000-0000-0000-0000-000000000001。参照(NoSQL 版)。[1]
- Cosmos DB Built-in Data Contributor — ID: 00000000-0000-0000-0000-000000000002。参照(NoSQL / Table 版)。[1][2]
- これらは Cosmos DB の「データプレーン」ロール定義であり、Azure ポータル上の通常の Azure RBAC (Microsoft.Authorization) の組み込みロールとは別扱いです(割当時は Microsoft.DocumentDB の API を使う等の注意点あり)。[3]
参考:
- Azure Cosmos DB for NoSQL — Data plane built-in roles reference(Cosmos DB Built-in Data Reader / Contributor の ID を明記)。[1]
- Azure Cosmos DB for Table — Data plane built-in roles reference(同 ID を掲載)。[2]
- Cosmos DB のデータプレーン RBAC の割当説明(roleDefinition の取得/割当方法の例)。[3]
必要なら、あなたの環境で実際に roleDefinition を取得する az / PowerShell コマンド例と確認手順を出しますか?
README: .env に Makefile が参照する Azure 環境変数を追記してください(デプロイ失敗防止)
Makefile の deploy-azure ターゲットが AZURE_RESOURCE_GROUP と AZURE_LOCATION を参照しているため、README の前準備でこれらを .env に明記して下さい。検索では AZURE_TENANT_ID / ARM_SUBSCRIPTION_ID は infra/** や Makefile から見つかりませんでした。roleDefinitionId 00000000-0000-0000-0000-000000000002 は Cosmos DB Built-in Data Contributor の正しい ID です。参照箇所: Makefile (deploy-azure) — --resource-group (line 189, 193), --location (line 190).
🤖 Prompt for AI Agents
In README.md around lines 219 to 232, the pre-setup instructions omit
environment variables referenced by the Makefile's deploy-azure target; update
the .env instructions to include AZURE_RESOURCE_GROUP and AZURE_LOCATION (with
example values or placeholders) so the Makefile can run without failing, and
retain the note that AZURE_TENANT_ID and ARM_SUBSCRIPTION_ID were not found in
infra/** or the Makefile; also keep the existing roleDefinitionId comment as-is.
User description
関連Issue / Related Issue
Closes #
変更の種類 / Type of Change
変更内容 / Description of Changes
テスト・動作確認
手動テスト手順 / Manual Testing Steps
その他 / Additional Notes
チェックリスト / Checklist
PR Type
Documentation
Description
Expand architecture and DI documentation
Clarify testing setup and directories
Update Docker/Dev Container instructions
Add Azure deployment workflow details
Diagram Walkthrough
File Walkthrough
CLAUDE.md
Elaborate architecture, DI, testing; add make restartCLAUDE.md
make restartcommand.README.md
Clarify run/dev workflow and Azure deployment stepsREADME.md
Summary by CodeRabbit