Skip to content
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

SeedsShell initial commit #20

Closed
wants to merge 1 commit into from

Conversation

TamiasSibiricus
Copy link

Data seeding shell #19

If this logic acceptable, i can make some improvements, doc comments and maybe tests..

@TamiasSibiricus
Copy link
Author

https://github.com/iteam-pro/seeds - sample plugin with data files. Currently can't be instaled by composer.

@rchavik
Copy link
Member

rchavik commented Oct 30, 2014

My wishlist:

  • Per migration data files (as I understand it now, loading a file is an all or nothing scenario)

    I hope we can have Migrations/10xxx_dosomething.php and its corresponding Data/10xxx_dosomething.php as the seed file

  • Data file generation utility that scans a table and writes them into the data migration files.

@markstory
Copy link
Member

I wouldn't make the data properties. PHP's syntax limitations around properties are pretty annoying. It might be better if the data is returned from a method. That would allow any userland data loads to use functions, classes, etc.

@bcrowe
Copy link
Contributor

bcrowe commented Oct 30, 2014

I don't see any usefulness in this unless @rchavik 's comments would be implemented. Furthermore, there should probably be some sort of export option available. It's not practical to maintain/create a bunch of "seed" files for a variable amount of database schemas. If exporting per migration doesn't exist, it's just as practical to use something like https://github.com/fzaninotto/Faker and create/update your own shell. Not sure the logistics with something like this co-existing with Faker, but, potentially a cool idea.

@markstory
Copy link
Member

I can see 'seed' data being useful in a few scenarios:

  • MySQL enums are dumb, so I frequently use constraint tables that have a few rows in them.
  • Some apps need operational data like built-in oauth applications, or enabled payment gateways, or user roles seeds are a good option for that.

@TamiasSibiricus
Copy link
Author

@markstory I think about methods and agree with you.. Also data in methods could be translatable to other languages.

@rchavik I don't see now any reason to relate seeding data to migrations. It is enough that the seeding data will conform to the database schema at the time of the last migration. As i know seeding data uses only for inserting default values like roles in setup phase. Also data seeding process runs only after migrations. So any changes for existing data you can do inside migrations and relate it to specific migration.

@bcrowe Nice idea to write Faker wrapper plugin for CakePHP. Thanks for link.

@josegonzalez
Copy link
Member

@bcrowe in the past, I've used schema annotations to generate faker fixtures for tests. Works reasonably well.

@rchavik
Copy link
Member

rchavik commented Oct 31, 2014

I think faker is not relevant in this case. We're loading real application data, not test data.

@TamiasSibiricus I linked you a real world use case of the scenario in #19.

In https://github.com/croogo/croogo/blob/master/Settings/Config/Migration/1392779470_setting_updates.php#L40-L50. I'm adding/updating three config keys: 'Site.locale', 'Site.admin_theme', 'Site.home_url',.

In order to update those three values in the migrations file, I have to write boring boiler plate save code which this PR can easily avoid.

Then I'd have to repeat the same thing again in the future migrations should I need to add new config keys.

Wasn't the purpose of this PR (among others) is to avoid repeating process?

@TamiasSibiricus
Copy link
Author

@rchavik i completely understand you and i know you are smart coder(i use Croogo 2 years ago for 1-2 projects). But.. I have some experience with Rails. Most of Rails applications positioned migration process and data seeding process as two separate processes. Migrations change db and/or data in db if it exists and changes needed. In this case data changers placed inside migrations. As i said later data seeder place data only after migrations and this action runs at once only in setup process. and seed data in most situations match db schema. I know Rails is not CakePHP, but some trick from Rails really useful.

Anyway i like to hear @markstory and @lorenzo opinions.

@markstory
Copy link
Member

Having seeds be a special kind of migration seems like a good compromise. I don't think we want to try and bend phinx to track seeds and migrations. However if instead we had easy to use tools to make data migrations that might cover most use cases. For example, a few cases I can think of are loading a yaml/json/toml/csv file, and calling a function to generate data. A subclass that provides tools for common cases would be enough in my opinion.

@ravage84
Copy link
Member

ravage84 commented Nov 1, 2014

Having seeds be a special kind of migration seems like a good compromise.

👍

A subclass that provides tools for common cases would be enough in my opinion.

👍

@TamiasSibiricus
Copy link
Author

@markstory as i understand your proposal is to keep data in [ yaml | json | toml | csv ] files, pik up data from this files and inject it to database with possible transformation of this data by subclass?

@markstory
Copy link
Member

@TamiasSibiricus Yeah something like:

<?php
class MyMigration extends DataSeedMigration {
  public function getDataFile() {
    return 'data/my-seed.csv';
  }
}

While we don't have to include all the formats at the beginning, having json/csv as a starting point might be nice.

@josegonzalez
Copy link
Member

@TamiasSibiricus any chance you can update your PR with the provided feedback?

@TamiasSibiricus
Copy link
Author

@josegonzalez Currently(i really thinking about this in continue of this month) i do not have any idea how to implement feedbacks into real code. Also i do not completely agree with opinion to mix migrations with data. I still think that data seeding process must run after migration processes to populate database.

@josegonzalez
Copy link
Member

Okay, let me think about this, as I think it's an important feature to have :)

@TamiasSibiricus
Copy link
Author

@josegonzalez Sure. Maybe it is reasonable to close this PR and open issue with link to this PR to discuss future seeding functions?

@josegonzalez
Copy link
Member

Good call.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants