Skip to content

Speed up ci#1580

Open
inochidainini wants to merge 10 commits intomainfrom
speedUpCi
Open

Speed up ci#1580
inochidainini wants to merge 10 commits intomainfrom
speedUpCi

Conversation

@inochidainini
Copy link
Copy Markdown
Contributor

@inochidainini inochidainini commented May 5, 2026

#1348
docker-composeでアプリを立ち上げていたのをやめciの中でatlasとtblsをそれぞれ実行するよう変更

Summary by CodeRabbit

  • Chores
    • Refactored CI workflow to use MariaDB service container directly instead of Docker Compose, improving pipeline efficiency
    • Updated project dependencies including database drivers, cloud SDKs, and development tools to latest compatible versions
    • Enhanced CI configuration with explicit migration directory settings

@inochidainini inochidainini requested a review from a team as a code owner May 5, 2026 12:43
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 5, 2026

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0b874bcd-32ef-4623-a4a3-1f31f29362ba

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR refactors the tbls GitHub Actions job to run directly in the workflow runner using a native MariaDB service container instead of Docker Compose, updates Go module dependencies (including Google Cloud, OpenTelemetry, and tool modules), and adds explicit migration directory configuration to the Atlas CI environment.

Changes

CI Pipeline Refactoring

Layer / File(s) Summary
Dependency Updates
go.mod
Bumped indirect dependencies: go-sqlite3 to v1.14.38, go-mssqldb to v1.9.8, and updated Google Cloud, OpenTelemetry, and golang.org/x/* packages. Added tool dependencies for oapi-codegen/v2/cmd/oapi-codegen and go.uber.org/mock/mockgen.
Atlas CI Configuration
atlas.ci.hcl
Added migration { dir = "file://migrations" } block within env "ci" to explicitly specify the migrations directory for Atlas in the CI environment.
Workflow Refactoring
.github/workflows/ci.yaml
Replaced Docker Compose execution with native MariaDB 10.6.4 service container (mariadb:10.6.4) configured with health-checks. Job steps now perform: code checkout, Go setup, Atlas installation via curl, database health-check polling loop using mysqladmin ping, migration application via atlas migrate apply --env ci, and documentation generation with go tool tbls doc against the service database.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Hops of joy from Docker's yoke,
Now MariaDB flows as native folk,
Health checks dance, migrations flow,
Atlas and tbls steal the show!
Dependencies upgraded high,
CI pipeline reaches for the sky!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Speed up ci' directly aligns with the main objective of refactoring the CI workflow to improve performance by removing docker-compose overhead and running services directly.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch speedUpCi

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 5, 2026

Migrate lint ✅

Lint output

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a 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)
atlas.ci.hcl (1)

21-33: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add url field to ci environment — atlas migrate apply requires it.

The ci environment in atlas.ci.hcl is missing the url field. Atlas HCL distinguishes between url (the target database for migrations) and dev (the dev database for schema normalization). The CI workflow runs atlas migrate apply --config file://atlas.ci.hcl --env ci without passing --url, so it depends on the url field in the config. Without it, the command will fail.

Since the MariaDB service in the CI job is the only database available, both url and dev can 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) vs mariadb:10 (floating minor) in migrate-lint.

Using a different MariaDB tag than the migrate-lint job 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

📥 Commits

Reviewing files that changed from the base of the PR and between 557e05c and 92e79cf.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (3)
  • .github/workflows/ci.yaml
  • atlas.ci.hcl
  • go.mod

Comment thread .github/workflows/ci.yaml
Comment on lines 159 to +174
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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

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.

Suggested change
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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

妥当な指摘だと思うので対応をお願いします

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mathsuky ご確認ありがとうございます! tbls ジョブの health-check パラメータを migrate-lint ジョブに合わせて修正します。

  • .github/workflows/ci.yamltbls ジョブの mariadb サービスの health-check オプションを migrate-lint ジョブと同様の設定に変更する

Failed to handle agent chat message. Please try again.

Copy link
Copy Markdown
Contributor

@mathsuky mathsuky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ありがとうございます!一旦1つコメントしました
CIが落ちているのでそちらにも対応をお願いします

Comment thread .github/workflows/ci.yaml
Comment on lines -169 to -172
if [[ -n $(git status --porcelain) ]]; then
echo "tbls diff detected. Please generate and commit the new docs. (task tbls)"
exit 1
fi
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

この部分が単が置き換えなどではなく単に消えてしまっているように見えます

Comment thread .github/workflows/ci.yaml
Comment on lines 159 to +174
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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

妥当な指摘だと思うので対応をお願いします

@inochidainini
Copy link
Copy Markdown
Contributor Author

ありがとうございます!一旦1つコメントしました CIが落ちているのでそちらにも対応をお願いします

了解です、対応しました。
ご指摘ありがとうございます。

@codecov
Copy link
Copy Markdown

codecov Bot commented May 10, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 51.73%. Comparing base (b539e29) to head (be02b5d).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1580   +/-   ##
=======================================
  Coverage   51.73%   51.73%           
=======================================
  Files         145      145           
  Lines       10814    10814           
=======================================
  Hits         5595     5595           
  Misses       4891     4891           
  Partials      328      328           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants