-
Notifications
You must be signed in to change notification settings - Fork 121
Add seed tracking. #939
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: 5.x
Are you sure you want to change the base?
Add seed tracking. #939
Changes from all commits
8e35170
fbe3526
4841a7b
a986999
510bfd0
299996f
159f7ef
b23c614
1f9a62e
d1ddd58
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 |
|---|---|---|
|
|
@@ -39,7 +39,7 @@ class SeedCommand extends Command | |
| */ | ||
| public static function defaultName(): string | ||
| { | ||
| return 'migrations seed'; | ||
| return 'seeds run'; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about the old command? How does that continue working?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They dont It is also simple enough IMO to understand. Those are simple commands that people can overwrite locally if they want to retain the old commands.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added a migration guide and a guide on how to easily restore the old way if needed. |
||
| } | ||
|
|
||
| /** | ||
|
|
@@ -55,10 +55,10 @@ public function buildOptionParser(ConsoleOptionParser $parser): ConsoleOptionPar | |
| '', | ||
| 'Runs a seeder script that can populate the database with data, or run mutations:', | ||
| '', | ||
| '<info>migrations seed Posts</info>', | ||
| '<info>migrations seed Users,Posts</info>', | ||
| '<info>migrations seed --plugin Demo</info>', | ||
| '<info>migrations seed --connection secondary</info>', | ||
| '<info>seeds run Posts</info>', | ||
| '<info>seeds run Users,Posts</info>', | ||
| '<info>seeds run --plugin Demo</info>', | ||
| '<info>seeds run --connection secondary</info>', | ||
| '', | ||
| 'Runs all seeds if no seed names are specified. When running all seeds', | ||
| 'in an interactive terminal, a confirmation prompt is shown.', | ||
|
|
@@ -87,6 +87,11 @@ public function buildOptionParser(ConsoleOptionParser $parser): ConsoleOptionPar | |
| 'short' => 's', | ||
| 'default' => ConfigInterface::DEFAULT_SEED_FOLDER, | ||
| 'help' => 'The folder where your seeds are.', | ||
| ]) | ||
| ->addOption('force', [ | ||
| 'short' => 'f', | ||
| 'help' => 'Force re-running seeds that have already been executed', | ||
| 'boolean' => true, | ||
| ]); | ||
|
|
||
| return $parser; | ||
|
|
@@ -184,9 +189,13 @@ protected function executeSeeds(Arguments $args, ConsoleIo $io): ?int | |
| $io->out(' - ' . $seedName); | ||
| } | ||
| $io->out(''); | ||
| $io->out('<warning>Note:</warning> Seeds do not track execution state. They will run'); | ||
| $io->out('regardless of whether they have been executed before. Ensure your'); | ||
| $io->out('seeds are idempotent or manually verify they should be (re)run.'); | ||
| if (!(bool)$args->getOption('force')) { | ||
| $io->out('<info>Note:</info> Seeds that have already been executed will be skipped.'); | ||
| $io->out('Use --force to re-run seeds.'); | ||
| } else { | ||
| $io->out('<warning>Warning:</warning> Running with --force will re-execute all seeds,'); | ||
| $io->out('potentially creating duplicate data. Ensure your seeds are idempotent.'); | ||
| } | ||
| $io->out(''); | ||
|
|
||
| // Ask for confirmation | ||
|
|
@@ -199,11 +208,11 @@ protected function executeSeeds(Arguments $args, ConsoleIo $io): ?int | |
| } | ||
|
|
||
| // run all the seed(ers) | ||
| $manager->seed(); | ||
| $manager->seed(null, (bool)$args->getOption('force')); | ||
| } else { | ||
| // run seed(ers) specified as arguments | ||
| foreach ($seeds as $seed) { | ||
| $manager->seed(trim($seed)); | ||
| $manager->seed(trim($seed), (bool)$args->getOption('force')); | ||
| } | ||
| } | ||
| $end = microtime(true); | ||
|
|
||
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.
What happens if a non-idempotent seed is run with the 'old' command? It seems like the result would be incorrect behavior.
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.
what old command? you mean migrations v4?
this is a new major, so any of this functionality is only available v5+.
PS: Thats already the case anway right now, it blindly runs everything all the time.
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.
Sure, but couldn't we retain at least a help message for the old command, or retain it with warnings? People likely have scripts, tooling and muscle memory that we'll be breaking.
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.
I would prefer we provide the guides I included and a clear release note for the major.
People dont accidentally upgrade a major afaik, so once they do, they can read and quickly enable if thats really something they need. Most can quickly adjust their tooling and will be happy not to have any additional noise and baggage on the CLI output for a year to come.