[#2379] Added 'reload-db2' ahoy command for migration database container.#2421
Merged
AlexSkrypnyk merged 7 commits intomainfrom Mar 25, 2026
Merged
[#2379] Added 'reload-db2' ahoy command for migration database container.#2421AlexSkrypnyk merged 7 commits intomainfrom
AlexSkrypnyk merged 7 commits intomainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds container-registry support for the migration DB image (VORTEX_DB2_IMAGE), a MigrationImage prompt handler and tests, makes Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Installer as Installer (PromptManager)
participant Env as .env / Config
participant Ahoy as Ahoy CLI
participant Docker as Docker Compose
participant DB as database2 container
User->>Installer: enable Migration + choose Container registry
Installer->>Installer: prompt MigrationImage, validate, discover
Installer->>Env: write VORTEX_DB2_IMAGE
User->>Ahoy: run `ahoy reload-db2`
Ahoy->>Docker: docker compose rm --force --stop --volumes database2
Ahoy->>Docker: docker compose up --build database2
Docker->>DB: start container with specified image
DB-->>Ahoy: container ready
Ahoy-->>User: command completes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 the current code and only fix it if needed.
Inline comments:
In @.ahoy.yml:
- Around line 166-168: Update the reload-db2 command so the confirmation prompt
explicitly references the migration database and remove the ineffective --build
flag when bringing up the database2 service: change the ahoy confirm message to
mention "migration database" (e.g., ahoy confirm "Running this command will
replace your migration database. Are you sure?") and replace the ahoy up
invocation that currently uses "-- --build database2" with a simple ahoy up --
database2 (or equivalent without --build) to avoid passing a no-op build flag
for the image-backed service.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e0aa3156-d5cd-43ab-8f89-92cf994d1311
⛔ Files ignored due to path filters (8)
.vortex/installer/tests/Fixtures/handler_process/migration_download_source_acquia/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_ftp/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_lagoon/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_s3/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_url/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled_circleci/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled_lagoon/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (3)
.ahoy.yml.vortex/tests/phpunit/Functional/AhoyWorkflowTest.php.vortex/tests/phpunit/Traits/Subtests/SubtestAhoyTrait.php
Comment on lines
+166
to
+168
| ahoy confirm "Running this command will replace your current database. Are you sure?" || exit 0 | ||
| docker compose rm --force --stop --volumes database2 | ||
| ahoy up -- --build database2 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspect reload-db2 command in .ahoy.yml"
rg -n -C2 'reload-db2|--build database2|replace your current database' .ahoy.yml
echo
echo "Inspect database2 service definition in docker-compose.yml"
awk '
/^database2:/ {in_block=1}
in_block {print}
in_block && /^ #;> MIGRATION/ {exit}
' docker-compose.yml
echo
echo "Expectation:"
echo "- database2 block contains image:"
echo "- database2 block does NOT contain build:"
echo "- then --build is a no-op for that service"Repository: drevops/vortex
Length of output: 963
🏁 Script executed:
# Find and read the database2 service definition in docker-compose.yml
rg -A 20 "^database2:" docker-compose.ymlRepository: drevops/vortex
Length of output: 40
🏁 Script executed:
# First, check if docker-compose.yml exists and find database2 anywhere in it
echo "=== Checking for docker-compose.yml files ==="
find . -name "docker-compose*.yml" -o -name "docker-compose*.yaml" | head -20
echo
echo "=== Searching for database2 in docker-compose files ==="
rg "database2" -l docker-compose* 2>/dev/null || echo "No files matched"
echo
echo "=== Searching for database2 anywhere ==="
rg "database2" --type yaml 2>/dev/null | head -30Repository: drevops/vortex
Length of output: 2770
🏁 Script executed:
# Read docker-compose.yml and extract the database2 service definition
rg -A 30 "^database2:" docker-compose.ymlRepository: drevops/vortex
Length of output: 40
🏁 Script executed:
# Read the docker-compose.yml file and extract database2 service
cat docker-compose.yml | grep -A 30 "database2:" | head -40Repository: drevops/vortex
Length of output: 1209
The reload-db2 command has two issues that should be corrected.
Line 166 should explicitly mention the migration database in the confirmation prompt, and Line 168's --build flag is ineffective since database2 (which uses image: uselagoon/mysql-8.4:26.2.0) has no build: definition.
Suggested fix
- ahoy confirm "Running this command will replace your current database. Are you sure?" || exit 0
+ ahoy confirm "Running this command will replace your current migration database. Are you sure?" || exit 0
docker compose rm --force --stop --volumes database2
- ahoy up -- --build database2
+ ahoy up -- database2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.ahoy.yml around lines 166 - 168, Update the reload-db2 command so the
confirmation prompt explicitly references the migration database and remove the
ineffective --build flag when bringing up the database2 service: change the ahoy
confirm message to mention "migration database" (e.g., ahoy confirm "Running
this command will replace your migration database. Are you sure?") and replace
the ahoy up invocation that currently uses "-- --build database2" with a simple
ahoy up -- database2 (or equivalent without --build) to avoid passing a no-op
build flag for the image-backed service.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2421 +/- ##
==========================================
- Coverage 79.64% 79.14% -0.51%
==========================================
Files 126 120 -6
Lines 6761 6642 -119
Branches 44 0 -44
==========================================
- Hits 5385 5257 -128
- Misses 1376 1385 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.vortex/installer/src/Prompts/Handlers/MigrationImage.php:
- Around line 30-37: Extract the duplicated migration-image formatting into a
single helper (e.g., a private static method like buildMigrationImageTemplate or
migrationImageTemplate) and update both placeholder(array $responses) and the
default-generating method (the one around lines 60-70) to call this helper; the
helper should accept the two inputs pulled from $responses using
OrgMachineName::id() and MachineName::id(), apply Converter::phpNamespace and
strtolower, and return the sprintf string ('E.g. %s/%s-data-migration:latest' or
the corresponding default value) so the formatting logic exists in one place and
both methods delegate to it.
In @.vortex/tests/phpunit/Functional/AhoyWorkflowTest.php:
- Around line 309-311: The test currently logs "Verify migration database image
configuration" and asserts the VORTEX_DB2_IMAGE value via
assertFileContainsString; add an additional assertion to verify the migration DB
workflow source by asserting the .env contains
"VORTEX_DOWNLOAD_DB2_SOURCE=container_registry" (use the same pattern and helper
used in this file—e.g., alongside the existing call in the same test method that
calls $this->logSubstep and $this->assertFileContainsString—to ensure the
selected path is validated end-to-end).
In @.vortex/tests/phpunit/Traits/Subtests/SubtestAhoyTrait.php:
- Line 907: The interactive call $this->cmd('ahoy reload-db2', txt: '`ahoy
reload-db2` reloads migration database container', tio: 60) can hang in CI;
update the SubtestAhoyTrait to make this invocation non-interactive by supplying
confirmation input or an explicit non-interactive flag. Locate the $this->cmd
call for 'ahoy reload-db2' and either pass a confirmation input (e.g., simulate
"y\n" via the cmd helper's input parameter) or add the appropriate ahoy flag
(e.g., --yes/--force) so the command completes without prompting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b44ee997-5c77-45ae-aaa7-cf80363888d2
⛔ Files ignored due to path filters (22)
.vortex/installer/tests/Fixtures/handler_process/migration_download_source_acquia/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/.gitignoreis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/tests/phpunit/Drupal/DatabaseSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/tests/phpunit/Drupal/SettingsTestCase.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/web/modules/custom/ys_migrate/migrations/ys_migrate_categories.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/web/modules/custom/ys_migrate/ys_migrate.info.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/web/sites/default/settings.migration.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/web/sites/default/settings.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_ftp/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_lagoon/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_s3/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_url/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled_circleci/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled_lagoon/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (12)
.env.vortex/installer/src/Prompts/Handlers/MigrationDownloadSource.php.vortex/installer/src/Prompts/Handlers/MigrationImage.php.vortex/installer/src/Prompts/PromptManager.php.vortex/installer/tests/Functional/Handlers/MigrationDownloadSourceHandlerProcessTest.php.vortex/installer/tests/Unit/Handlers/AbstractHandlerDiscoveryTestCase.php.vortex/installer/tests/Unit/Handlers/AbstractHandlerTypeTest.php.vortex/installer/tests/Unit/Handlers/MigrationImageHandlerDiscoveryTest.php.vortex/installer/tests/Unit/Schema/SchemaGeneratorTest.php.vortex/tests/phpunit/Functional/AhoyWorkflowTest.php.vortex/tests/phpunit/Traits/Subtests/SubtestAhoyTrait.phpdocker-compose.yml
Comment on lines
+30
to
+37
| public function placeholder(array $responses): ?string { | ||
| if (isset($responses[OrgMachineName::id()]) && isset($responses[MachineName::id()]) | ||
| && !empty($responses[OrgMachineName::id()]) && !empty($responses[MachineName::id()])) { | ||
| return sprintf('E.g. %s/%s-data-migration:latest', | ||
| strtolower(Converter::phpNamespace($responses[OrgMachineName::id()])), | ||
| strtolower(Converter::phpNamespace($responses[MachineName::id()])) | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Extract shared migration-image template generation.
The placeholder and default paths duplicate the same formatting logic; consolidate into one helper to prevent drift.
Refactor sketch
class MigrationImage extends AbstractHandler {
+ private function buildMigrationImage(array $responses): ?string {
+ if (
+ isset($responses[OrgMachineName::id()], $responses[MachineName::id()]) &&
+ !empty($responses[OrgMachineName::id()]) &&
+ !empty($responses[MachineName::id()])
+ ) {
+ return sprintf(
+ '%s/%s-data-migration:latest',
+ strtolower(Converter::phpNamespace($responses[OrgMachineName::id()])),
+ strtolower(Converter::phpNamespace($responses[MachineName::id()]))
+ );
+ }
+ return NULL;
+ }
+
public function placeholder(array $responses): ?string {
- if (isset($responses[OrgMachineName::id()]) && isset($responses[MachineName::id()])
- && !empty($responses[OrgMachineName::id()]) && !empty($responses[MachineName::id()])) {
- return sprintf('E.g. %s/%s-data-migration:latest',
- strtolower(Converter::phpNamespace($responses[OrgMachineName::id()])),
- strtolower(Converter::phpNamespace($responses[MachineName::id()]))
- );
- }
+ $image = $this->buildMigrationImage($responses);
+ if ($image !== NULL) {
+ return sprintf('E.g. %s', $image);
+ }
return parent::placeholder($responses);
}
public function default(array $responses): null|string|bool|array {
- if (
- isset($responses[OrgMachineName::id()]) &&
- isset($responses[MachineName::id()]) &&
- !empty($responses[OrgMachineName::id()]) &&
- !empty($responses[MachineName::id()])
- ) {
- return sprintf(
- '%s/%s-data-migration:latest',
- strtolower(Converter::phpNamespace($responses[OrgMachineName::id()])),
- strtolower(Converter::phpNamespace($responses[MachineName::id()]))
- );
- }
-
- return NULL;
+ return $this->buildMigrationImage($responses);
}Also applies to: 60-70
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.vortex/installer/src/Prompts/Handlers/MigrationImage.php around lines 30 -
37, Extract the duplicated migration-image formatting into a single helper
(e.g., a private static method like buildMigrationImageTemplate or
migrationImageTemplate) and update both placeholder(array $responses) and the
default-generating method (the one around lines 60-70) to call this helper; the
helper should accept the two inputs pulled from $responses using
OrgMachineName::id() and MachineName::id(), apply Converter::phpNamespace and
strtolower, and return the sprintf string ('E.g. %s/%s-data-migration:latest' or
the corresponding default value) so the formatting logic exists in one place and
both methods delegate to it.
| ); | ||
|
|
||
| $this->logSubstep('Reload database2 from the container image'); | ||
| $this->cmd('ahoy reload-db2', txt: '`ahoy reload-db2` reloads migration database container', tio: 60); |
There was a problem hiding this comment.
Provide confirmation input for ahoy reload-db2 in test flow.
At Line 907, this command is interactive. Without explicit input, the subtest can hang or timeout in non-interactive contexts.
💡 Proposed fix
- $this->cmd('ahoy reload-db2', txt: '`ahoy reload-db2` reloads migration database container', tio: 60);
+ $this->cmd('ahoy reload-db2', inp: ['y'], txt: '`ahoy reload-db2` reloads migration database container', tio: 60);📝 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
| $this->cmd('ahoy reload-db2', txt: '`ahoy reload-db2` reloads migration database container', tio: 60); | |
| $this->cmd('ahoy reload-db2', inp: ['y'], txt: '`ahoy reload-db2` reloads migration database container', tio: 60); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.vortex/tests/phpunit/Traits/Subtests/SubtestAhoyTrait.php at line 907, The
interactive call $this->cmd('ahoy reload-db2', txt: '`ahoy reload-db2` reloads
migration database container', tio: 60) can hang in CI; update the
SubtestAhoyTrait to make this invocation non-interactive by supplying
confirmation input or an explicit non-interactive flag. Locate the $this->cmd
call for 'ahoy reload-db2' and either pass a confirmation input (e.g., simulate
"y\n" via the cmd helper's input parameter) or add the appropriate ahoy flag
(e.g., --yes/--force) so the command completes without prompting.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
AlexSkrypnyk
force-pushed
the
feature/2379-reload-db2
branch
from
930c30a to
14f4f12
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docker-compose.yml (1)
201-213:⚠️ Potential issue | 🟡 MinorReplace
--buildflag with explicitdocker compose pullinreload-db2command.The
database2service uses a directimage:directive without a build context, so the--buildflag in thereload-db2command has no effect (Docker Compose silently skips building image-only services). This differs fromreload-db, which usesdocker compose build databaseon a build-based service.For proper image updates—especially with private registries—the command should explicitly pull the image:
docker compose pull database2Either add this step before starting the container, or remove the ineffective
--buildflag.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-compose.yml` around lines 201 - 213, The reload-db2 task is using a useless --build flag for the image-only service database2; update the reload-db2 command to explicitly pull the image before starting by running "docker compose pull database2" (or remove the --build flag and add a separate "docker compose pull database2" step) so the database2 image from the registry is refreshed; locate the reload-db2 recipe that targets the database2 service and replace the ineffective build behavior with an explicit pull of the database2 image.
♻️ Duplicate comments (4)
.vortex/installer/src/Prompts/Handlers/MigrationImage.php (1)
30-40: 🧹 Nitpick | 🔵 TrivialExtract shared migration-image template generation.
The placeholder and default methods duplicate the same formatting logic for building the image name string. This was previously flagged and should be consolidated into a helper method to prevent drift.
♻️ Proposed refactor
class MigrationImage extends AbstractHandler { + + private function buildMigrationImage(array $responses): ?string { + if ( + isset($responses[OrgMachineName::id()], $responses[MachineName::id()]) && + !empty($responses[OrgMachineName::id()]) && + !empty($responses[MachineName::id()]) + ) { + return sprintf( + '%s/%s-data-migration:latest', + strtolower(Converter::phpNamespace($responses[OrgMachineName::id()])), + strtolower(Converter::phpNamespace($responses[MachineName::id()])) + ); + } + return NULL; + } public function placeholder(array $responses): ?string { - if (isset($responses[OrgMachineName::id()]) && isset($responses[MachineName::id()]) - && !empty($responses[OrgMachineName::id()]) && !empty($responses[MachineName::id()])) { - return sprintf('E.g. %s/%s-data-migration:latest', - strtolower(Converter::phpNamespace($responses[OrgMachineName::id()])), - strtolower(Converter::phpNamespace($responses[MachineName::id()])) - ); - } - - return parent::placeholder($responses); + $image = $this->buildMigrationImage($responses); + return $image !== NULL ? sprintf('E.g. %s', $image) : parent::placeholder($responses); } public function default(array $responses): null|string|bool|array { - if ( - isset($responses[OrgMachineName::id()]) && - isset($responses[MachineName::id()]) && - !empty($responses[OrgMachineName::id()]) && - !empty($responses[MachineName::id()]) - ) { - return sprintf( - '%s/%s-data-migration:latest', - strtolower(Converter::phpNamespace($responses[OrgMachineName::id()])), - strtolower(Converter::phpNamespace($responses[MachineName::id()])) - ); - } - - return NULL; + return $this->buildMigrationImage($responses); }Also applies to: 59-74
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.vortex/installer/src/Prompts/Handlers/MigrationImage.php around lines 30 - 40, The placeholder and default methods in MigrationImage duplicate the same image-name formatting logic; extract that logic into a single private helper (e.g., buildMigrationImageTemplate or formatMigrationImage) and have both placeholder(array $responses) and default(array $responses) call it; the helper should accept the $responses array, check OrgMachineName::id() and MachineName::id() presence/emptiness, call Converter::phpNamespace and strtolower to produce the "%s/%s-data-migration:latest" string, and fall back to parent::placeholder($responses) or parent::default($responses) when inputs are missing..vortex/tests/phpunit/Traits/Subtests/SubtestAhoyTrait.php (1)
907-907:⚠️ Potential issue | 🟠 MajorProvide confirmation input for
ahoy reload-db2in test flow.The
ahoy reload-db2command prompts for confirmation. Without explicit input, this test could hang or timeout in non-interactive CI contexts.🐛 Proposed fix
- $this->cmd('ahoy reload-db2', txt: '`ahoy reload-db2` reloads migration database container', tio: 60); + $this->cmd('ahoy reload-db2', inp: ['y'], txt: '`ahoy reload-db2` reloads migration database container', tio: 60);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.vortex/tests/phpunit/Traits/Subtests/SubtestAhoyTrait.php at line 907, The test invokes an interactive command that prompts for confirmation but doesn't supply input, causing CI hangs; update the $this->cmd call for 'ahoy reload-db2' to provide the confirmation input (e.g., send "y\n" or "yes\n") so the prompt is answered non-interactively, for example by adding the appropriate input/stdin parameter to the existing $this->cmd('ahoy reload-db2', txt: '`ahoy reload-db2` reloads migration database container', tio: 60) call in the SubtestAhoyTrait..vortex/tests/phpunit/Functional/AhoyWorkflowTest.php (1)
309-311:⚠️ Potential issue | 🟡 MinorAdd explicit migration source assertion for end-to-end validation.
At Line 310, the test verifies image value, but it should also verify that
.envselected thecontainer_registrysource path.Suggested patch
$this->logSubstep('Verify migration database image configuration'); + $this->assertFileContainsString('.env', 'VORTEX_DOWNLOAD_DB2_SOURCE=container_registry', '.env should contain migration download source'); $this->assertFileContainsString('.env', 'VORTEX_DB2_IMAGE=' . self::VORTEX_DB_IMAGE_TEST, '.env should contain migration database image');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.vortex/tests/phpunit/Functional/AhoyWorkflowTest.php around lines 309 - 311, The test currently checks only the migration DB image value using assertFileContainsString in AhoyWorkflowTest::logSubstep/assertFileContainsString; add an explicit assertion that the .env also contains the migration source path key set to container_registry (e.g. verify presence of "VORTEX_DB2_SOURCE=container_registry") using the same assertFileContainsString helper so the end-to-end test validates both image and source selection..ahoy.yml (1)
166-166:⚠️ Potential issue | 🟡 MinorClarify the confirmation prompt target.
Line 166 should explicitly mention the migration database to avoid ambiguity for
reload-db2.Suggested patch
- ahoy confirm "Running this command will replace your current database. Are you sure?" || exit 0 + ahoy confirm "Running this command will replace your current migration database. Are you sure?" || exit 0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.ahoy.yml at line 166, Update the confirmation prompt invoked by ahoy (the ahoy confirm call) to explicitly reference the migration database being replaced (e.g., mention "migration database" or the specific DB name) so that the reload-db2 command’s intention is unambiguous; edit the string passed to ahoy confirm to include that explicit target (and any variable like MIGRATION_DB if used) so users know reload-db2 will replace the migration database.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@docker-compose.yml`:
- Around line 201-213: The reload-db2 task is using a useless --build flag for
the image-only service database2; update the reload-db2 command to explicitly
pull the image before starting by running "docker compose pull database2" (or
remove the --build flag and add a separate "docker compose pull database2" step)
so the database2 image from the registry is refreshed; locate the reload-db2
recipe that targets the database2 service and replace the ineffective build
behavior with an explicit pull of the database2 image.
---
Duplicate comments:
In @.ahoy.yml:
- Line 166: Update the confirmation prompt invoked by ahoy (the ahoy confirm
call) to explicitly reference the migration database being replaced (e.g.,
mention "migration database" or the specific DB name) so that the reload-db2
command’s intention is unambiguous; edit the string passed to ahoy confirm to
include that explicit target (and any variable like MIGRATION_DB if used) so
users know reload-db2 will replace the migration database.
In @.vortex/installer/src/Prompts/Handlers/MigrationImage.php:
- Around line 30-40: The placeholder and default methods in MigrationImage
duplicate the same image-name formatting logic; extract that logic into a single
private helper (e.g., buildMigrationImageTemplate or formatMigrationImage) and
have both placeholder(array $responses) and default(array $responses) call it;
the helper should accept the $responses array, check OrgMachineName::id() and
MachineName::id() presence/emptiness, call Converter::phpNamespace and
strtolower to produce the "%s/%s-data-migration:latest" string, and fall back to
parent::placeholder($responses) or parent::default($responses) when inputs are
missing.
In @.vortex/tests/phpunit/Functional/AhoyWorkflowTest.php:
- Around line 309-311: The test currently checks only the migration DB image
value using assertFileContainsString in
AhoyWorkflowTest::logSubstep/assertFileContainsString; add an explicit assertion
that the .env also contains the migration source path key set to
container_registry (e.g. verify presence of
"VORTEX_DB2_SOURCE=container_registry") using the same assertFileContainsString
helper so the end-to-end test validates both image and source selection.
In @.vortex/tests/phpunit/Traits/Subtests/SubtestAhoyTrait.php:
- Line 907: The test invokes an interactive command that prompts for
confirmation but doesn't supply input, causing CI hangs; update the $this->cmd
call for 'ahoy reload-db2' to provide the confirmation input (e.g., send "y\n"
or "yes\n") so the prompt is answered non-interactively, for example by adding
the appropriate input/stdin parameter to the existing $this->cmd('ahoy
reload-db2', txt: '`ahoy reload-db2` reloads migration database container', tio:
60) call in the SubtestAhoyTrait.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 78c79bdc-8772-411c-b0aa-9f3d0f7fb842
⛔ Files ignored due to path filters (30)
.vortex/installer/tests/Fixtures/handler_process/migration_download_source_acquia/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_acquia/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/.gitignoreis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/tests/phpunit/Drupal/DatabaseSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/tests/phpunit/Drupal/SettingsTestCase.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/web/modules/custom/ys_migrate/migrations/ys_migrate_categories.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/web/modules/custom/ys_migrate/ys_migrate.info.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/web/sites/default/settings.migration.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/web/sites/default/settings.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_ftp/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_ftp/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_lagoon/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_lagoon/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_s3/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_s3/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_url/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_url/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled_circleci/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled_circleci/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled_lagoon/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled_lagoon/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (13)
.ahoy.yml.env.vortex/installer/src/Prompts/Handlers/MigrationDownloadSource.php.vortex/installer/src/Prompts/Handlers/MigrationImage.php.vortex/installer/src/Prompts/PromptManager.php.vortex/installer/tests/Functional/Handlers/MigrationDownloadSourceHandlerProcessTest.php.vortex/installer/tests/Unit/Handlers/AbstractHandlerDiscoveryTestCase.php.vortex/installer/tests/Unit/Handlers/AbstractHandlerTypeTest.php.vortex/installer/tests/Unit/Handlers/MigrationImageHandlerDiscoveryTest.php.vortex/installer/tests/Unit/Schema/SchemaGeneratorTest.php.vortex/tests/phpunit/Functional/AhoyWorkflowTest.php.vortex/tests/phpunit/Traits/Subtests/SubtestAhoyTrait.phpdocker-compose.yml
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
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 |
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 #2379
Summary
Added a
reload-db2ahoy command that allows developers to reload the migration database container using the local database image. This mirrors the existingreload-dbcommand but targets thedatabase2container used for migration workflows. The command was added to the main.ahoy.yml, all relevant installer fixture snapshots, and covered by a new functional subtest.Changes
.ahoy.ymlreload-db2command (under#;< MIGRATION/#;> MIGRATIONguards) that stops and removes thedatabase2container, then rebuilds and restarts it viaahoy up -- --build database2. Includes a confirmation prompt before proceeding.Installer fixture snapshots (all migration-related
.ahoy.ymlfixtures updated identically):migration_download_source_acquia,migration_download_source_ftp,migration_download_source_lagoon,migration_download_source_s3,migration_download_source_url,migration_enabled,migration_enabled_circleci,migration_enabled_lagoonreload-db2command block alongside the already-presentdownload-db2block.Tests
SubtestAhoyTrait.php: Added assertion that.ahoy.ymlcontainsreload-db2insidesubtestAhoyMigrationSetup, plus a newsubtestAhoyMigrationReloadDb()method that exercisesahoy reload-db2and verifies that provisioning with migration succeeds after the reload.AhoyWorkflowTest.php: WiredsubtestAhoyMigrationReloadDb()into the migration workflow test sequence.Before / After
Summary by CodeRabbit
New Features
Tests