Hi Guys
I found today a problem after release version 1.11.0 of propel-orm https://github.com/spryker/propel-orm/releases
The version tag 1.11.0 required the propel version alpha10. This version has a bug in propel migrate console command https://github.com/propelorm/Propel2/compare/2.0.0-alpha8...2.0.0-alpha10#diff-1ccdce29404b8984d1b2101f0069a564
In MigrationMigrateCommand they return now CODE_ERROR
instead false
at line 84 but for symfony command vendor/symfony/console/Command/Command.php:262
false was a success and now the command is aborted.
@valerii.trots can u please check internal how we can fix this? Normally propel have to be fixed. But Spryker has offical release propel-orm version 1.11 that required propel alpha 10 version
The problem now is that our install and deployment scripts just fail due the exit code of migration command console even if no migrations were executed.
Has anyone from today the same problem? One possible solution is of course to constraint the spryker/propel-orm
version to 1.10 in composer.json.
According to our development, you need to adjust the code on your side:
The proper error code here is returned (as per Symfony specs), as such their code should be adjusted rather. Symfony public function run(InputInterface $input, OutputInterface $output) @return int The command exit code is the contract. And will in the future be enforced, as such the change is needed. <https://github.com/spryker/propel-orm/releases/tag/1.11.0> will clarify this?
Yes of course.. but the return code is wrong...
In the case there were not migration to be executed then propel alpha8 just return false
Now alpha10 has to be compatible with symfony 5 that enforce to return int. OK. Clear..
But the return of symfony command is the follwowing:
return is_numeric($statusCode) ? (int) $statusCode : 0;
and as u see false
means (for backward compatibility) SUCCESS
Now Migration Command Console in case no migration needed return 1
instead false
and this for symfony command is ERROR
and not more SUCCESS
Propel alpha10 has a bug. They just replace the return false
with return CODE_ERROR
but this is not correct in every case, aboveall in the migraiton command console in the case no migration needed.. Especially here:
if (!$manager->getFirstUpMigrationTimestamp()) { $output->writeln('All migrations were already executed - nothing to migrate.'); return static::CODE_ERROR; }
before here was return false!
@valerii.trots if u want i can discuss about it with the developer that said that i need to adjust the code on my side
No worries, let's play ping pong with them. 🙂
🙂 ok.. just investigate in details the changes
alpha8...alpha10 and together with \Symfony\Component\Console\Command\Command::run return statement
some news?
<https://github.com/propelorm/Propel2/pull/1595/files#diff-1ccdce29404b8984d1b2101f0069a564R84> is the change, and return false was supposed to make a 1 exit code, so changing to 1 directly is actually the correct change. The fact that this actually should have returned true (and thus now 0) is a different topic. He might be right, and this should be fixed. But how than this be a regression if it was always wrong so far? I opened a PR for our AA to discuss: <https://github.com/propelorm/Propel2/pull/1617>
You can forward this to the project, they can +1 it if thats correct and hopefully we can include this in the next alpha release.
ok, danke!