Magento 2 Recipe: Fix import needed after deployment failure during upgrade #4180#4181
Magento 2 Recipe: Fix import needed after deployment failure during upgrade #4180#4181carlos-reynosa wants to merge 17 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a dedicated Magento 2 post-failure hook so that additional recovery steps (config import + maintenance disable) run when a deployment fails—intended to keep Magento configuration consistent after a failed upgrade/deploy attempt.
Changes:
- Replaces the direct
magento:maintenance:disablefailure hook with a new grouped taskdeploy:magento:failed. - Adds
deploy:magento:failedtask to runmagento:config:importand then disable maintenance mode. - Regenerates Magento 2 recipe documentation to include the new task and updated source line references.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
recipe/magento2.php |
Introduces deploy:magento:failed and wires it to deploy:failed for Magento-specific recovery steps. |
docs/recipe/magento2.md |
Updates autogenerated docs to reflect the new task and shifted source line numbers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Remove unneeded comment for docs. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
This update fixes Magento config import behavior after failed deployments. -Added a dedicated current-release config check and import flow for failure scenarios. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
recipe/magento2.php:347
magento:config:import:on-currentdirectly runsbin/magentofrom{{current_path}}without the directory-exists guard used by the maintenance tasks. In failure scenarios (especially first deploy), this can throw and abort subsequent failure handling. Consider adding the sameif [ -d {{current_path}} ]guard and/or catchingRunExceptionso this task becomes a no-op when there is no live release to import from.
desc('Config Import on current release');
task('magento:config:import:on-current', function () {
if (get('config_import_needed_on_current')) {
// do not use {{bin/magento}} as it must run on the current (last successful) release in failure scenarios
run('{{bin/php}} {{current_path}}/{{magento_dir}}/bin/magento app:config:import --no-interaction');
} else {
writeln('App config is up to date => import skipped');
}
});
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
recipe/magento2.php:359
magento:config:import:on-currentonly catchesRunException, butrun()can throwTimeoutExceptionor other non-Run exceptions. If that happens, this task will fail and (becausedeploy:magento:failedis a group task)magento:maintenance:disablemay never run, potentially leaving the site in maintenance mode after a failed deploy. Consider catching\Throwable(orTimeoutException+RunException) and swallowing it here since this is a best-effort failure hook.
desc('Config Import on current release');
task('magento:config:import:on-current', function () {
try {
if (get('config_import_needed_on_current')) {
// do not use {{bin/magento}} as it must run on the current (last successful) release in failure scenarios
run('{{bin/php}} {{current_path}}/{{magento_dir}}/bin/magento app:config:import --no-interaction');
} else {
writeln('App config import skipped');
}
} catch (RunException $e) {
writeln('Unable to import app config on current release => import skipped');
}
});
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
recipe/magento2.php:354
magento:config:import:on-currentrunsapp:config:importusing the globaldefault_timeout. If users set a very largedefault_timeoutand the import hangs,deploy:failedcan be blocked for a long time and delay recovery (including disabling maintenance mode). Consider passing an explicittimeout/idleTimeoutfor this command so the failure handler stays best-effort and bounded.
if (get('config_import_needed_on_current')) {
// do not use {{bin/magento}} as it must run on the current (last successful) release in failure scenarios
run('{{bin/php}} {{current_path}}/{{magento_dir}}/bin/magento app:config:import --no-interaction');
} else {
writeln('App config import skipped');
| try { | ||
| // detect if app:config:import is needed on the current (live) release | ||
| // do not use {{bin/magento}} as it resolves via release_or_current_path which is unreliable in failure scenarios | ||
| run('{{bin/php}} {{current_path}}/{{magento_dir}}/bin/magento app:config:status'); |
There was a problem hiding this comment.
Probably overkill unless we start adding it to every run command. I suggest not.
Pull request to ensure that if the magento deployment fails and the release is reverted the configuration is properly imported. Deployments mail fail during upgrade and after new configuration has been imported.
Fixes #4180