Update the Migrations class to support the seeding feature #182
Conversation
This gives the ability to run seed from a non-shell context, using the Migrations\Migrations class.
| @@ -18,7 +18,6 @@ public function run() | |||
| { | |||
| $data = [ | |||
| [ | |||
| 'id' => '1', | |||
There was a problem hiding this comment.
Can we not include primary keys in seed data?
There was a problem hiding this comment.
I think it is better to not manually add the primary key. SQL server, for example does not like that, whereas Postgres will not advance the sequence automatically
There was a problem hiding this comment.
Wow really? Do you have documentation for those two cases? I think it might be important to note for users creating seed data.
Note: I've not had that issue on mysql, but mysql is the php of databases so...
There was a problem hiding this comment.
It is a documented feature of those databases. It is possible to workaround those cases, but it requires specific code and special configuration for each of the drivers
There was a problem hiding this comment.
We can. Tests pass without problems with the change.
How it will behave is tied to the schema of your database : it will be no problem if you have this column defined as autoincrement (which is the case here).
All examples of seed in phinx doc doesn't include a value for the PK fields (well, I suppose there is a PK since there's no schema provided)
There was a problem hiding this comment.
I made the change to test the "call the same seeder multiple times" case, because I realized that with the PK defined, the values were not added on top of each other (it did an update).
There was a problem hiding this comment.
How do we build relational fixtures then?
There was a problem hiding this comment.
Technically, you can do it.
The tests from PR #181 passed with the PK hard-coded for all db drivers (however, we do not test against SQLServer).
But according to @lorenzo, you are probably going to need some special code depending on the db driver you are using.
Apparently for Postgres, you will have to manually update the sequence index to the next value - something postgres does automatically if you do not provide a value for a PK sequence in an INSERT.
As @josegonzalez said, maybe it would be a good thing to add a paragraph in the README / doc about this as it might confuse users.
For instance, I did not even know about this limitation for SQL Server and Postgres. From my experience, it does not cause any trouble with MySQL, so I thought it would not with the others...
|
good job again @HavokInspiration :) |
Update the Migrations class to support the seeding feature
|
@HavokInspiration Are we ready to tag a new release? |
| * Sets the command | ||
| * | ||
| * @param string $command Command name to store. | ||
| * @return self |
There was a problem hiding this comment.
So far we have been using $this.
But I would favor if we migrated to self for CakePHP in the near future.
|
@lorenzo Yes, the seed feature is ready as far as I'm concerned |
This gives the ability to run seed from a non-shell context, using the
Migrations\Migrationsclass.Follow up to #179 and #181