peterjaap
commented
2 months ago Thanks for the PR!
Changing setup:upgrade to the two separate upgrade commands is a great idea.
However, I don't see this working;
run("{{bin/php}} {{bin/magento}} setup:db-data:upgrade --no-interaction")->once();Afaik run() returns a string (the output of the command) and not an object to set once() on. This should be wrapped in a separate task(), which can take once(). Right, @antonmedv ?
As a sidenote, we ran into accidentally enabling non-listed modules error with setup:upgrade before. To avoid this, we created a small PHP tool to check this before the deployment task starts in our pipeline;
<?php
$output = [];
exec('php bin/magento module:status --disabled', $output, $exitCode);
// If No disabled modules, exit with exit code 1
if ($exitCode === 1) {
exit(0);
}
$config = include 'app/etc/config.php';
$whitelistedDisabledModules = $config['whitelisted_disabled_modules'] ?? [];
$messages = [];
foreach ($output as $module) {
if (!in_array($module, $whitelistedDisabledModules)) {
$messages[] = $module . ' is disabled in modules but not whitelisted under whitelisted_disabled_modules in app/etc/config.php.';
}
}
if (!empty($messages)) {
foreach ($messages as $message) {
echo $message . PHP_EOL;
}
exit(1);
} else {
exit(0);
}Maybe you'll find it useful too.
antonmedv
dverkade
This PR addresses two issues:
setup:upgradehowever this command does more then only upgrading the database. It can change the sort order of the modules listed in app/etc/config.php or if a module is not listed in that file it will enable that module. IMHO deployments should be consistent, and the app/etc/config.php shouldn't be changed during deployment. That's why I changed this into two commands that actually only update the DB schema and DB data. I've borrowed this from the Magento 2 Capistrano implementation, which already does it like this: https://github.com/davidalger/capistrano-magento2/blob/master/lib/capistrano/tasks/magento.rake#L261