Speed up ci#1580
Conversation
atlas.ci.hclを参照するよう変更
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR refactors the ChangesCI Pipeline Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
Migrate lint ✅ Lint output |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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)
atlas.ci.hcl (1)
21-33:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
urlfield tocienvironment —atlas migrate applyrequires it.The
cienvironment inatlas.ci.hclis missing theurlfield. Atlas HCL distinguishes betweenurl(the target database for migrations) anddev(the dev database for schema normalization). The CI workflow runsatlas migrate apply --config file://atlas.ci.hcl --env ciwithout passing--url, so it depends on theurlfield in the config. Without it, the command will fail.Since the MariaDB service in the CI job is the only database available, both
urlanddevcan point to the same connection string.Proposed fix
env "ci" { dev = "mysql://root:pass@localhost:3306/trap_collection" + url = "mysql://root:pass@localhost:3306/trap_collection" migration { dir = "file://migrations" }🤖 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 `@atlas.ci.hcl` around lines 21 - 33, Add a missing "url" field to the env "ci" block so Atlas has a target DB for migrate apply; inside the env "ci" stanza (the same block that currently defines dev, migration, and lint) set url to the same connection string as dev (e.g., url = "mysql://root:pass@localhost:3306/trap_collection") so both url and dev point to the CI MariaDB instance.
🧹 Nitpick comments (1)
.github/workflows/ci.yaml (1)
161-162: 💤 Low value
mariadb:10.6.4(pinned patch) vsmariadb:10(floating minor) inmigrate-lint.Using a different MariaDB tag than the
migrate-lintjob can surface version-specific SQL dialect differences between the two CI checks. Consider aligning both jobs to the same image tag.🤖 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 @.github/workflows/ci.yaml around lines 161 - 162, The mariadb service in this CI job is pinned to image: mariadb:10.6.4 while the migrate-lint job uses a different tag (mariadb:10); update the mariadb service image so both jobs use the same tag (either change image: mariadb:10.6.4 to image: mariadb:10 to match migrate-lint, or change migrate-lint to the specific 10.6.4 tag) to ensure consistent SQL dialect behavior across the CI checks; locate the mariadb service definition and the migrate-lint job image references to make the change.
🤖 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 @.github/workflows/ci.yaml:
- Around line 159-174: The tbls mariadb service health-check is set to an
excessively high retry count; in the mariadb service options block (service name
"mariadb" under "tbls") replace "--health-retries 1000" with a realistic value
such as "--health-retries 10" and align the timing with migrate-lint by changing
"--health-interval 1s" to "--health-interval 10s" and adding a
"--health-start-period 30s" (or similar start-period) so the job fails fast on
real failures instead of waiting ~16 minutes.
---
Outside diff comments:
In `@atlas.ci.hcl`:
- Around line 21-33: Add a missing "url" field to the env "ci" block so Atlas
has a target DB for migrate apply; inside the env "ci" stanza (the same block
that currently defines dev, migration, and lint) set url to the same connection
string as dev (e.g., url = "mysql://root:pass@localhost:3306/trap_collection")
so both url and dev point to the CI MariaDB instance.
---
Nitpick comments:
In @.github/workflows/ci.yaml:
- Around line 161-162: The mariadb service in this CI job is pinned to image:
mariadb:10.6.4 while the migrate-lint job uses a different tag (mariadb:10);
update the mariadb service image so both jobs use the same tag (either change
image: mariadb:10.6.4 to image: mariadb:10 to match migrate-lint, or change
migrate-lint to the specific 10.6.4 tag) to ensure consistent SQL dialect
behavior across the CI checks; locate the mariadb service definition and the
migrate-lint job image references to make the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d8475bd8-1905-483a-825b-ee202e7b0c11
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
.github/workflows/ci.yamlatlas.ci.hclgo.mod
| tbls: | ||
| services: | ||
| mariadb: | ||
| image: mariadb:10.6.4 | ||
| env: | ||
| MYSQL_ROOT_PASSWORD: pass | ||
| MYSQL_DATABASE: trap_collection | ||
| TZ: Asia/Tokyo | ||
| ports: | ||
| - 3306:3306 | ||
| options: >- | ||
| --health-cmd "mysqladmin ping -h 127.0.0.1 -ppass" | ||
| --health-interval 1s | ||
| --health-timeout 5m | ||
| --health-retries 1000 | ||
|
|
There was a problem hiding this comment.
Health-check --health-retries 1000 is excessively high — a DB startup failure would block for ~16 minutes.
At --health-interval 1s × 1000 retries, the job won't give up for roughly 16 minutes if the container never becomes healthy. The migrate-lint job uses --health-retries 10 (with --health-interval 10s and a --health-start-period). Bringing tbls in line prevents silent stalls.
🐛 Proposed fix — align with migrate-lint health-check tuning
options: >-
--health-cmd "mysqladmin ping -h 127.0.0.1 -ppass"
- --health-interval 1s
- --health-timeout 5m
- --health-retries 1000
+ --health-interval 10s
+ --health-start-period 10s
+ --health-timeout 5s
+ --health-retries 10📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| tbls: | |
| services: | |
| mariadb: | |
| image: mariadb:10.6.4 | |
| env: | |
| MYSQL_ROOT_PASSWORD: pass | |
| MYSQL_DATABASE: trap_collection | |
| TZ: Asia/Tokyo | |
| ports: | |
| - 3306:3306 | |
| options: >- | |
| --health-cmd "mysqladmin ping -h 127.0.0.1 -ppass" | |
| --health-interval 1s | |
| --health-timeout 5m | |
| --health-retries 1000 | |
| tbls: | |
| services: | |
| mariadb: | |
| image: mariadb:10.6.4 | |
| env: | |
| MYSQL_ROOT_PASSWORD: pass | |
| MYSQL_DATABASE: trap_collection | |
| TZ: Asia/Tokyo | |
| ports: | |
| - 3306:3306 | |
| options: >- | |
| --health-cmd "mysqladmin ping -h 127.0.0.1 -ppass" | |
| --health-interval 10s | |
| --health-start-period 10s | |
| --health-timeout 5s | |
| --health-retries 10 |
🤖 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 @.github/workflows/ci.yaml around lines 159 - 174, The tbls mariadb service
health-check is set to an excessively high retry count; in the mariadb service
options block (service name "mariadb" under "tbls") replace "--health-retries
1000" with a realistic value such as "--health-retries 10" and align the timing
with migrate-lint by changing "--health-interval 1s" to "--health-interval 10s"
and adding a "--health-start-period 30s" (or similar start-period) so the job
fails fast on real failures instead of waiting ~16 minutes.
There was a problem hiding this comment.
@mathsuky ご確認ありがとうございます！ tbls ジョブの health-check パラメータを migrate-lint ジョブに合わせて修正します。
.github/workflows/ci.yamlのtblsジョブの mariadb サービスの health-check オプションをmigrate-lintジョブと同様の設定に変更する
Failed to handle agent chat message. Please try again.
mathsuky
left a comment
mathsuky
left a comment
There was a problem hiding this comment.
ありがとうございます!一旦1つコメントしました
CIが落ちているのでそちらにも対応をお願いします
| if [[ -n $(git status --porcelain) ]]; then | ||
| echo "tbls diff detected. Please generate and commit the new docs. (task tbls)" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
この部分が単が置き換えなどではなく単に消えてしまっているように見えます
| tbls: | ||
| services: | ||
| mariadb: | ||
| image: mariadb:10.6.4 | ||
| env: | ||
| MYSQL_ROOT_PASSWORD: pass | ||
| MYSQL_DATABASE: trap_collection | ||
| TZ: Asia/Tokyo | ||
| ports: | ||
| - 3306:3306 | ||
| options: >- | ||
| --health-cmd "mysqladmin ping -h 127.0.0.1 -ppass" | ||
| --health-interval 1s | ||
| --health-timeout 5m | ||
| --health-retries 1000 | ||
|
|
Check diff追加
了解です、対応しました。 |
tablewriterをv0.0.5に
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1580 +/- ##
=======================================
Coverage 51.43% 51.43%
=======================================
Files 146 146
Lines 11157 11157
=======================================
Hits 5739 5739
Misses 5079 5079
Partials 339 339 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
mathsuky
left a comment
mathsuky
left a comment
There was a problem hiding this comment.
ありがとうございます!
コメントしたので確認をお願いいたします:bow-nya2:
また，CIがどのくらい高速化したかも測定したいので，それは今度集会で揃った時にでもやりましょう!
| run: docker compose -f docker/tbls/compose.yaml up --build --abort-on-container-exit | ||
| run: | | ||
| TBLS_DSN=mariadb://root:pass@127.0.0.1:3306/trap_collection \ | ||
| go tool tbls doc --rm-dist |
There was a problem hiding this comment.
go tool tbls doc --rm-dist -c .tbls.ymlのように-cオプションを付すとドキュメント関連の大量の差分が消えると思うので試してみてください
2 participants
#1348
docker-composeでアプリを立ち上げていたのをやめciの中でatlasとtblsをそれぞれ実行するよう変更
Summary by CodeRabbit