-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Magento 2 Recipe: Fix import needed after deployment failure during upgrade #4180 #4181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -370,7 +370,14 @@ function magentoDeployAssetsSplit(string $area) | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| after('deploy:symlink', 'magento:cache:flush'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| after('deploy:failed', 'magento:maintenance:disable'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| after('deploy:failed', 'deploy:magento:failed'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| //Run Magento Deployment Failure Tasks | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| desc('Run magento post deployment failure tasks.'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| desc('Run magento post deployment failure tasks.'); | |
| desc('Run Magento post-deployment failure tasks'); |
Copilot
AI
Apr 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deploy:magento:failed runs magento:config:import, but that task resolves {{bin/magento}} via {{release_or_current_path}}. On failures that occur before deploy:symlink, the {{deploy_path}}/release symlink still exists and points to the new/failed release (see recipe/deploy/release.php + recipe/deploy/symlink.php), so the config status/import will run against the failed release instead of the still-active current release—this doesn’t restore configuration after a failed upgrade.
Consider making the failure path explicitly run config status/import against {{current_path}} (or otherwise ensure release_or_current_path resolves to current_path here) and guard it so the deploy:failed hook remains best-effort even if Magento isn’t runnable in the release directory.
| task('deploy:magento:failed', [ | |
| 'magento:config:import', | |
| 'magento:maintenance:disable', | |
| ]); | |
| task('deploy:magento:failed', function () { | |
| $hadReleaseOrCurrentPath = false; | |
| $originalReleaseOrCurrentPath = null; | |
| if (has('release_or_current_path')) { | |
| $hadReleaseOrCurrentPath = true; | |
| $originalReleaseOrCurrentPath = get('release_or_current_path'); | |
| } | |
| // Ensure that config import on failure is run against the active current release, | |
| // not the (possibly broken) new release. | |
| set('release_or_current_path', '{{current_path}}'); | |
| try { | |
| invoke('magento:config:import'); | |
| } catch (RunException $exception) { | |
| // Best-effort: ignore failures during rollback config import. | |
| } catch (GracefulShutdownException $exception) { | |
| // Best-effort: ignore failures during rollback config import. | |
| } | |
| // Restore original release_or_current_path configuration if it existed before. | |
| if ($hadReleaseOrCurrentPath) { | |
| set('release_or_current_path', $originalReleaseOrCurrentPath); | |
| } | |
| try { | |
| invoke('magento:maintenance:disable'); | |
| } catch (RunException $exception) { | |
| // Best-effort: ignore failures when disabling maintenance mode on failure. | |
| } catch (GracefulShutdownException $exception) { | |
| // Best-effort: ignore failures when disabling maintenance mode on failure. | |
| } | |
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The inline
//Run Magento Deployment Failure Taskscomment is being pulled into the autogenerated docs as a separate paragraph (see the new “Run Magento Deployment Failure Tasks” line indocs/recipe/magento2.md), which makes the docs a bit redundant/noisy. Consider either removing this comment or converting it into a clearerdesc(...)/documentation text so docgen output stays clean.