-
-
Notifications
You must be signed in to change notification settings - Fork 212
feat: add --dry-run option #512
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: 4.2.x
Are you sure you want to change the base?
Conversation
I think this should rather be done by:
It could look like this: final readonly class DryRunORMExecutor extends AbstractExecutor
{
use ORMExecutorCommon;
/** @inheritDoc */
public function execute(array $fixtures, bool $append = false): void
{
$executor = $this;
$this->em->beginTransaction();
try {
if ($append === false) {
$executor->purge();
}
foreach ($fixtures as $fixture) {
$executor->load($em, $fixture);
}
$this->em->flush();
} finally {
$this->em->close();
$this->em->rollBack();
}
}
} That way, we avoid nesting transactions ( |
Thank you for your response @greg0ire. So it would be something like this:
if ($input->getOption('dry-run')) {
$ui->text(' <comment>(dry-run)</comment>');
$executor = new DryRunORMExecutor($em, $purger);
} else {
$executor = new ORMExecutor($em, $purger);
} If that's the best solution, I'll implement it. But doesn't that create repetitive code between Wouldn’t it be possible to just modify Something like this:
final class ORMExecutor extends AbstractExecutor
{
use ORMExecutorCommon;
/** @inheritDoc */
public function execute(array $fixtures, bool $append = false, bool $dryRun = false): void
{
$executor = $this;
$this->em->beginTransaction();
try {
if ($append === false) {
$executor->purge();
}
foreach ($fixtures as $fixture) {
$executor->load($this->em, $fixture);
}
$this->em->flush();
if ($dryRun === true) {
$this->em->rollBack();
} else {
$this->em->commit();
}
} catch (\Throwable $e) {
$this->em->rollBack();
} finally {
$this->em->close();
}
}
}
$executor->execute($fixtures, $input->getOption('append'), $input->getOption('dry-run'));
/**
* Executes the given array of data fixtures.
*
* @param FixtureInterface[] $fixtures Array of fixtures to execute.
* @param bool $append Whether to append the data fixtures or purge the database before loading.
* @param bool $dryRun Whether to simulate the execution of the fixtures without making actual changes to the database.
*/
abstract public function execute(array $fixtures, bool $append = false, bool $dryRun = false): void; |
So you would change all the children classes of |
Yes, or a third parameter |
Common code could be moved in I prefer my solution. If we continue piling up more booleans, the complexity of the method is going to skyrocket. |
@greg0ire |
Add
--dry-run
option todoctrine:fixtures:load
command to display loaded fixtures without actually loading themcloses: #511 #508