Add Seibu Bus GTFS support#1530
Merged
Merged
Conversation
github-actions
Bot
added
feature
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughこのPRは、GTFS バス取り込みシステムを東京交通局（Toei）のみから西武バス（Seibu）を含む複数フィード対応へ拡張します。新しい環境変数 Changes複数フィード対応の GTFS バス取り込みシステム
Sequence DiagramsequenceDiagram
participant Env as ".env / .env.local"
participant Compose as "docker-compose"
participant API as "API Container"
participant Importer as "import_gtfs"
participant Downloader as "download_gtfs"
participant ODPT as "ODPT API"
participant DB as "Database"
Env->>Compose: env 読み込み (ENABLE_EXPERIMENTAL_BUS_FEATURE / ODPT_ACCESS_TOKEN)
Compose->>API: 環境変数注入
API->>Importer: import_gtfs 実行
Importer->>Downloader: spawn_blocking で各 feed の download_gtfs
Downloader->>ODPT: GET (token を含む場合あり)
Downloader->>Importer: ZIP 展開完了
Importer->>DB: トランザクション内でフィード毎に import_* を実行・INSERT
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
stationapi/src/import.rs (1)
357-362: ⚡ Quick win
spawn_blockingが直列実行になっており、複数フィード時の起動時間を不必要に伸ばしています。
for内で即awaitしているため、実際には並列ダウンロードになっていません。ハンドルを溜めて最後に join してください。差分案
- for feed in enabled_feeds.iter().copied() { - tokio::task::spawn_blocking(move || download_gtfs(feed)) - .await - .map_err(|e| format!("Failed to spawn blocking task: {}", e))? - .map_err(|e| -> Box<dyn std::error::Error> { e })?; - } + let mut handles = Vec::with_capacity(enabled_feeds.len()); + for feed in enabled_feeds.iter().copied() { + handles.push(tokio::task::spawn_blocking(move || download_gtfs(feed))); + } + for handle in handles { + handle + .await + .map_err(|e| format!("Failed to spawn blocking task: {}", e))? + .map_err(|e| -> Box<dyn std::error::Error> { e })?; + }Based on learnings: "Prioritize quality and performance over implementation speed. Always favor code quality and runtime performance over velocity, considering algorithmic complexity and opportunities to replace O(n×m) linear scans with O(n+m) indexed lookups."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stationapi/src/import.rs` around lines 357 - 362, The loop currently awaits each tokio::task::spawn_blocking immediately, causing serial execution; instead, spawn all tasks first and collect their JoinHandle<Result<_, Box<dyn Error>>> (the handles created by tokio::task::spawn_blocking calling download_gtfs), then await them all later (e.g., via futures::future::join_all or iterating the handles) and map JoinError and the inner Result into the same error handling you already do. Locate the loop using enabled_feeds.iter().copied(), the spawn_blocking(...) call and the download_gtfs function; change it to push each spawn_blocking handle into a Vec, then after the loop await all handles and propagate/map both spawn join errors and download_gtfs errors exactly as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@stationapi/src/import.rs`:
- Around line 1612-1618: The current company_cd_for_gtfs_route(route_id: &str)
silently defaults unknown prefixes to Toei (119); change it to return
Option<i32> (or Result<i32, Error>) instead of a hardcoded i32 so
unknown/unsupported prefixes are reported to the caller; update
company_cd_for_gtfs_route to return Some(253) for "seibu:" and None for anything
else (or Err with a clear message), and then update its callers (the import path
that maps GTFS routes) to skip or explicitly error when None/Err is returned so
unknown prefixes are not ingested as 119.
---
Nitpick comments:
In `@stationapi/src/import.rs`:
- Around line 357-362: The loop currently awaits each
tokio::task::spawn_blocking immediately, causing serial execution; instead,
spawn all tasks first and collect their JoinHandle<Result<_, Box<dyn Error>>>
(the handles created by tokio::task::spawn_blocking calling download_gtfs), then
await them all later (e.g., via futures::future::join_all or iterating the
handles) and map JoinError and the inner Result into the same error handling you
already do. Locate the loop using enabled_feeds.iter().copied(), the
spawn_blocking(...) call and the download_gtfs function; change it to push each
spawn_blocking handle into a Vec, then after the loop await all handles and
propagate/map both spawn join errors and download_gtfs errors exactly as before.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2c63251a-2419-4668-8f64-c3d4105faf7d
⛔ Files ignored due to path filters (1)
data/1!companies.csvis excluded by!**/*.csv
📒 Files selected for processing (5)
.envAGENTS.mdcompose.ymldocs/architecture.mdstationapi/src/import.rs
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
西武バスのGTFSフィードをODPT経由で取り込めるようにしました。既定では従来どおり都営バスのみを使用し、
ENABLE_EXPERIMENTAL_BUS_FEATURE=trueの場合に西武バスを含む全設定済みGTFSフィードを使用します。変更の種類
変更内容
toei:/seibu:の名前空間を付与しました。ODPT_ACCESS_TOKENから取得するようにしました。ENABLE_EXPERIMENTAL_BUS_FEATUREを追加し、未設定または偽の場合は都営バスのみ、真の場合は全GTFSフィードを使用するようにしました。#1f63c6を使用してlines.line_color_cのNOT NULL制約に違反しないようにしました。テスト
cargo fmt --all -- --checkが通ることcargo clippy -- -D warningsが通ることcargo test（SQLX_OFFLINE=true）が通ること追加で実行した確認:
cargo fmtcargo run -p data_validatorcargo check -p stationapi関連Issue
なし
スクリーンショット（任意）
なし
Summary by CodeRabbit
リリースノート
New Features
Documentation
Chores