[#2718] Used 'export-db' for the Lagoon pre-deployment database backup.#2724
Merged
Conversation
The 'export-db' router now runs the file export in place when not on a Docker host, so the Lagoon backup routes through it instead of calling 'export-db-file' directly. Host detection is overridable via 'RUN_ON_HOST' for tests.
WalkthroughThe production backup step now calls ChangesDB export routing
Sequence Diagram(s)sequenceDiagram
participant post_rollout as "post-rollout task"
participant export_db as "export-db"
participant docker_cmd as "docker"
participant docker_compose as "docker compose"
participant export_db_file as "export-db-file"
participant export_db_image as "export-db-image"
participant deploy_container_registry as "deploy-container-registry"
post_rollout->>export_db: run backup command
export_db->>docker_cmd: check availability on PATH
alt host
export_db->>docker_compose: compose exec -T ... export-db-file
docker_compose->>export_db_file: export database dump
opt VORTEX_EXPORT_DB_IMAGE is set
export_db->>export_db_image: export database image
opt VORTEX_EXPORT_DB_CONTAINER_REGISTRY_DEPLOY_PROCEED=1
export_db->>deploy_container_registry: deploy image
end
end
else non-host
export_db->>export_db_file: export database file directly
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
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 docstrings
🧪 Generate unit tests (beta)
Comment |
![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
🤖 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 @.vortex/tooling/tests/unit/export-db.bats:
- Around line 54-63: The export-db test is not actually exercising Docker-based
host detection because RUN_ON_HOST may still be inherited, and is_host() in
.vortex/tooling/src/export-db will prefer that override first. Update the test
case in export-db.bats to explicitly clear RUN_ON_HOST before running
.vortex/tooling/src/export-db, so the assertion verifies the docker CLI
detection path and the mock_docker expectations remain valid.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 923cc2c6-f4c6-4092-8e01-466925662dcc
⛔ Files ignored due to path filters (5)
.vortex/installer/tests/Fixtures/handler_process/hosting_lagoon/.lagoon.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/hosting_project_name___lagoon/.lagoon.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_disabled_lagoon/.lagoon.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled_lagoon/.lagoon.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/provision_database_lagoon/.lagoon.ymlis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (3)
.lagoon.yml.vortex/tooling/src/export-db.vortex/tooling/tests/unit/export-db.bats
Comment on lines
+54
to
+63
| @test "export-db: Detects the host from the Docker CLI when the override is unset" { | ||
| pushd "${LOCAL_REPO_DIR}" >/dev/null || exit 1 | ||
|
|
||
| mock_docker=$(mock_command "docker") | ||
| mock_set_output "${mock_docker}" "Exported through detected Docker." 1 | ||
|
|
||
| run .vortex/tooling/src/export-db | ||
| assert_success | ||
| assert_output_contains "Exported through detected Docker." | ||
| assert_equal "1" "$(mock_get_call_num "${mock_docker}")" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Unset RUN_ON_HOST before asserting auto-detection behavior.
This test claims the override is unset, but it never clears RUN_ON_HOST. Since is_host prefers RUN_ON_HOST over docker detection (.vortex/tooling/src/export-db: is_host()), an inherited env var can bypass the path under test.
Suggested patch
`@test` "export-db: Detects the host from the Docker CLI when the override is unset" {
pushd "${LOCAL_REPO_DIR}" >/dev/null || exit 1
+ unset RUN_ON_HOST
mock_docker=$(mock_command "docker")
mock_set_output "${mock_docker}" "Exported through detected Docker." 1📝 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
| @test "export-db: Detects the host from the Docker CLI when the override is unset" { | |
| pushd "${LOCAL_REPO_DIR}" >/dev/null || exit 1 | |
| mock_docker=$(mock_command "docker") | |
| mock_set_output "${mock_docker}" "Exported through detected Docker." 1 | |
| run .vortex/tooling/src/export-db | |
| assert_success | |
| assert_output_contains "Exported through detected Docker." | |
| assert_equal "1" "$(mock_get_call_num "${mock_docker}")" | |
| `@test` "export-db: Detects the host from the Docker CLI when the override is unset" { | |
| pushd "${LOCAL_REPO_DIR}" >/dev/null || exit 1 | |
| unset RUN_ON_HOST | |
| mock_docker=$(mock_command "docker") | |
| mock_set_output "${mock_docker}" "Exported through detected Docker." 1 | |
| run .vortex/tooling/src/export-db | |
| assert_success | |
| assert_output_contains "Exported through detected Docker." | |
| assert_equal "1" "$(mock_get_call_num "${mock_docker}")" |
🤖 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 @.vortex/tooling/tests/unit/export-db.bats around lines 54 - 63, The
export-db test is not actually exercising Docker-based host detection because
RUN_ON_HOST may still be inherited, and is_host() in
.vortex/tooling/src/export-db will prefer that override first. Update the test
case in export-db.bats to explicitly clear RUN_ON_HOST before running
.vortex/tooling/src/export-db, so the assertion verifies the docker CLI
detection path and the mock_docker expectations remain valid.
|
Code coverage (threshold: 90%) Per-class coverage |
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
Member
Author
|
Code coverage (threshold: 90%) Per-class coverage |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2724 +/- ##
==========================================
- Coverage 86.67% 86.22% -0.45%
==========================================
Files 96 89 -7
Lines 4719 4560 -159
Branches 47 3 -44
==========================================
- Hits 4090 3932 -158
+ Misses 629 628 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
📖 Documentation preview for this pull request has been deployed to Netlify: https://6a3cb3148470f9d2b9aa1700--vortex-docs.netlify.app This preview is rebuilt on every commit and is not the production documentation site. |
This was referenced Jun 25, 2026
Merged
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.
Closes #2718
Summary
Switches the Lagoon pre-deployment database backup from calling
export-db-filedirectly to calling the publicexport-dbrouter. The router now detects whether it is running on the host (via aRUN_ON_HOSToverride or presence ofdockerinPATH) and dispatches accordingly: inside a Lagoon container (off-host) it runsexport-db-filein place without requiring Docker, while on the host it continues to usedocker compose execfor file exports orexport-db-imagefor image exports. Unit tests cover all dispatch paths.Changes
.lagoon.yml: replacedexport-db-filewithexport-dbin the pre-deployment backup task;VORTEX_DB_DIRpreserved, no new flags added..vortex/tooling/src/export-db: addedis_host()helper (honoursRUN_ON_HOSToverride, else probescommand -v docker); restructured dispatch so off-host runsexport-db-filein place and on-host continues the existing Docker Compose / image paths; removed the harddockerdependency check that blocked container-side execution..vortex/tooling/tests/unit/export-db.bats: new unit tests covering in-place file export (off-host), Docker Compose file export (on-host), auto-detection via Docker CLI, image export, and image export with registry deployment..lagoon.ymlchange.Before / After
Summary by CodeRabbit
New Features
Bug Fixes