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

migrations load dependencies like assets do #8202

Closed
Faryshta opened this issue Apr 27, 2015 · 48 comments
Closed

migrations load dependencies like assets do #8202

Faryshta opened this issue Apr 27, 2015 · 48 comments

Comments

@Faryshta
Copy link
Contributor

Lets see how assets load dependencies.

class AppAsset extends AssetBundle
{
    public $depends = [
        'yii\web\YiiAsset',
        'yii\bootstrap\BootstrapAsset',
    ];
}

This allows a tree-like loading of dependencies each one calling the resources and executing the actions it requires. Since each asset is recognized by the unique full class name (with the namespace) then each asset is unique and can be autoloaded without issues.

The way the migrations do it

class m150427_071254_user extends Migration
{
    // no more code
}

This approach uses the moment the migraiton was created to execute each dependency in chronological order, this implies several differences such as

  1. Classes can't be autoloaded

  2. There can't be a hard dependency that gets forced to be executed first

  3. There is no way to know which migration actually depends on another or force a hard dependency for example asking to execute a migration user_profile that add columns to the user table.

  4. a path is always required to be provided when executing the migrations.

  5. When executing migrate/down command there is no way to know which other migrations will be affected.

So i propose to have a remaking of this for 2.1 to something similar to

namespace Acme\Sihpments\migrations;

use yii\db\Migration;

class ShipmentLog extends Migration
{
    $depends = [
         'Acme\Shipments\migration\Shipment'
    ];
}

The migration ShipmentLog creates a shipment_log table for all the stages and dates of a shipment, The table requires a foreign key to link it to the shipment table defined on the Shipment migration.

This method is specially got for plugins or modules that extend the functionality of an existing table, example if i want to add columns to the user table defined on the yii migrations.

@samdark samdark added this to the 2.1.x milestone Apr 27, 2015
@klimov-paul
Copy link
Member

This does not make sense for me.

This approach uses the moment the migraiton was created to execute each dependency in chronological order

Yes, migration is a kind of version control system for your database. Each migration represent a particular state of the DB structure evolution. Of course it could be said that each particular migration may have a 'parent' and a 'child', but there is no sense of explicitely bind them as they are already ordred using timing principle.

  1. Classes can't be autoloaded

Just why do you need that? Class autoloading make sense in case you always know explicit name of the class you want to get. For the migration it does not work this way. Each migration has a unique name composed from time string. In order to find any new migration we need to scan thier source directory one way or another. None will tell migrate command which class name(s) are holding new migrations.

  1. There can't be a hard dependency that gets forced to be executed first

Why do you need that? This does not make any sense at all. Migration are always executed in timing order, ensuring composed database state will be stable. Why any dependency should be?
Even if it will be, how do you suggest to revert migration with dependencies? Revert all migrations, which this one depends on? And what if other migrations depends on those?

  1. a path is always required to be provided when executing the migrations.

As I said there is no way to find the new migrations unless scan thier source directory. Of course we can use a namespace for the migration classes and then may attempt to resolve migration path using Yii::getAlias(), but this will cost more trouble then benefit.

  1. When executing migrate/down command there is no way to know which other migrations will be affected.

What does this mean? When you run migrate/down it shows you EXACT names of the migrations, which will be reverted.

I suppose you mistake migration mechanism for something else. Maybe you should start from explanation of the problem(s), which you attempt ot resolve?

@xskif
Copy link
Contributor

xskif commented Apr 27, 2015

I think @Faryshta want to separate app into modules with ready to use migrations. And reuse modules in other apps, which require some control mechanism of migrations. Example:

  1. Start new app.
  2. add module User.
  3. Use user migration.
  4. add module Post.
  5. Use post migration.
  6. Write new module - Comments.
  7. Write Comments migration. and use it
  8. Something happens and i need to remove the Post module so i need to revert concrete migration, but i can't, because migrate/down reverts Comments migration.
  9. FAIL =)

@klimov-paul
Copy link
Member

That is why migrations are allowed to be separated by paths.
If you have several independent migration flows (like modules) you can separate them using different paths.
Module is meant to be an independent entity. If it holds some internal migrations they should be independed.

@xskif
Copy link
Contributor

xskif commented Apr 27, 2015

@klimov-paul , i knew that.
Can i revert the concrete migration for the concrete module if i have an independent migrations for each module?

@klimov-paul
Copy link
Member

So, you wish to have an ability to revert migration under particular path, like following:

yii migrate/down 1 --path='@mymodule/migrations'

Currently path is not stored in the migration table. However you can use different migration tables for different path:

yii migrate/down 1 --path='@mymodule/migrations' --migrationTable='MyModuleMigrations'

@Faryshta
Copy link
Contributor Author

I think the point here is that migrations are not linear in the real world.

For example i can write a migration today based on the yii2 rbac migration and i will just add a few functionalities.

Tomorrow a new and improved rbac is published by a third person, so i wanna change the dependency that my plugin no longer uses the yii2 migration and uses the other one instead... even if my migration was written before.

Another much more realistic case would be that the client wanna drop a functionality, add a new functionality, revert to a previous way to do the bussiness logic.

my options would be 1) screw the migrations and do it by hand. right now there is no way to know which migrations depends on each other

or 2) check by hand which migration affect which.

the current system discourage the use for migration for things like extensions, modules and plugins which want to add functionality to an existing migration. RBAC is a great example.

Now imagine with more complex systems like e-commerce. I can write my selling platform using a shopping cart from a 3rd developer. I would need to add steps on my documentation for each migration the shopping cart has.

now imagine that besides shopping cart i need product list, categories, tagging for each product, recomendations, alerts for product existances, etc, etc, etc. Each of them provided by different vendors.

well i can either duplicate the code or make a huge list of all the migration my platform will require to execute.

@Faryshta
Copy link
Contributor Author

Faryshta commented May 1, 2016

can we talk about this again since we are already preparing 2.1?

I can work on a pr but it will take me a lot of time and effort so I wanna know if I won't waste my time

@samdark
Copy link
Member

samdark commented May 1, 2016

There are multiple points and I agree with some:

  1. Migration class names could be changed to be PSR-compliant.
  2. Migrations could be namespaced. That would not save you from specifying path but may make it easier via specifying namespaces instead and relying on autoloading.
  3. Child-parent relationships of migrations... that's interesting but let's imagine a situation: we have app migrations and we need to add some third-party module migrations. How to do that? Module migrations, obviously, could not rely on app migrations.

@Faryshta
Copy link
Contributor Author

Faryshta commented May 1, 2016

we have app migrations and we need to add some third-party module migrations. How to do that?

We can let composer deal with it.

MyMigration extends BaseMigration
{
    public $extends = [
        \Third\Party\ShoppingCartMigration::class,
        \console\migrations\Init::class
    ];
}

we install the third party migrations using composer and let composer load the class

@Faryshta
Copy link
Contributor Author

@yiisoft/core-developers any update with this? can I start working on this for 2.1?

@samdark
Copy link
Member

samdark commented Jun 6, 2016

I don't know. That looks kinda complicated and fragile when we think about merging migrations from 10 branches developed at the same time.

@samdark
Copy link
Member

samdark commented Jun 6, 2016

↑ is about parent-child relationships i.e. in the case all 10 new migrations will specify the same migration as the parent since it's the only one available at the moment these are created. Then there will be a problem about which migration should be executed first.

@Faryshta
Copy link
Contributor Author

Faryshta commented Jun 7, 2016

@samdark what do you mean? like circular dependencies? Or when two migrations depend on a parent migration?

If I need migration A to be executed before migration B then I just need to add A to the $depends property in migration B

class B extends Migration
{
    public $depends= [A::class];
}

The same as fixtures and assets.

@samdark
Copy link
Member

samdark commented Jun 7, 2016

  • We have 3 people in a team. John, Dave, Bob.
  • Repository is using master branch for releases. Migrations from this branch are applied when deploying to production server.
  • Features are developed in separate branches and then merged to master.
  • Current migration is AddUser.
  • John, Dave, Bob are starting branches feature1, feature2, feature3.
  • Each feature requires a migration so it's created and depends on AddUser.
  • When all the branches are merged to master we have 3 migrations depending on AddUser so it's not clear in which order these should be executed.

The situation itself is OK. Logically dependency is correct but execution should be predictable and repeatable therefore we should define which order to use for migrations with equal dependencies.

@Faryshta
Copy link
Contributor Author

Faryshta commented Jun 7, 2016

@samdark how about having an internal constant?

class B extends Migration
{
     const CREATION_TIME = 1234567890; // unix timestamp
}

just first idea that came to mind.

@samdark
Copy link
Member

samdark commented Jun 7, 2016

We can use filemtime instead.

@rob006
Copy link
Contributor

rob006 commented Jun 7, 2016

Why change migrations names conventions in first place? If I have 20 migrations in one directory, it is better if they are sorted chronologically.

@Faryshta
Copy link
Contributor Author

Faryshta commented Jun 7, 2016

@rob006 mostly to be able to autoload them. As they are right now its impossible.

@samdark how do you propose it?

@rob006
Copy link
Contributor

rob006 commented Jun 7, 2016

As they are right now its impossible.

Why? Using namespace like app\migrations\m150416_155549_create_user_and_auth_tables will not work?

@nkostadinov
Copy link

One step at a time. I think the first thing is to add namespaces to migrations and make them PSR compliant. It will again require to specify migrationPath but when downgrading you wont have to and the classes will look more consistent.

@samdark
Copy link
Member

samdark commented Jun 7, 2016

@rob006 it's against PSR-2 which isn't really a good thing.

@rob006
Copy link
Contributor

rob006 commented Jun 7, 2016

app\migrations\M150416T155549CreateUserAndAuthTables?

@samdark
Copy link
Member

samdark commented Jun 7, 2016

Yes. That's better. Less human-readable but will be sorted right. That if we're talking about the old approach.

Overall the idea of @Faryshta is very interesting. At first I've denied it because it's different and I've got used to timestamp-based migrations but after more thinking it seems like a good approach. In fact migrations do depend on each other. You're starting new migration based on current database state so based on currently applied migration X. That solves some conflicting cases.

@Faryshta
Copy link
Contributor Author

Faryshta commented Jun 7, 2016

@samdark another issue is that sometimes you have to change your migrations order, if you add the timestamp on a constant/property/method then you can edit it later, on the filename its barely impossible without breaking something else, will work on a patch this weekend

@samdark
Copy link
Member

samdark commented Jun 7, 2016

Changing order after migrations got to repository sounds like a terrible idea to me.

@Faryshta
Copy link
Contributor Author

Faryshta commented Jun 8, 2016

lets say I made a table dinner_table and then I make a seemingly unrelated table waitress.

Then I get asked to associate each dinner_table to a waitress bu a waitress_id column.

On a developing stage is much easier to simply add the waitress_id column to the dinner_table migration and then run migrate/down, migrate/up than to make a new migration that depends on the previous ones.

its also cleaner when I have to check the migrations and run them for CI testing.

Again this is mostly for developing stage whent he customer usually changes the structure of the project at every meeting.

@rob006
Copy link
Contributor

rob006 commented Jun 8, 2016

You should create new migration that adds necessary changes and don't mess with migrations history. Your approach is unacceptable when you work with other people, because it creating problems that migration should solve.

@Faryshta
Copy link
Contributor Author

Faryshta commented Jun 8, 2016

Your approach is unacceptable when you work with other people, because it creating problems that migration should solve.

I send them an email 'syncronize migrations and run fixtures' and thats about the solution for my developing team. Yet again, this is for developing stages and much easier than "run yet another migration and see if the fixtures run"

@rob006
Copy link
Contributor

rob006 commented Jun 8, 2016

I send them an email 'syncronize migrations and run fixtures' and thats about the solution for my developing team.

This is exactly what I'm talking about. Migration are designed to update the database for the current state of project code. If you need another solution to synchronize database with current state of migrations you might as well not use them at all - use plain SQL dump or single migration with all db structure, and write migrations on release after development hell.

@samdark
Copy link
Member

samdark commented Jun 8, 2016

Here I agree with @rob006. If migration was pushed and it works, that's it. It's set in stone and should never be touched.

@Faryshta
Copy link
Contributor Author

Faryshta commented Jun 9, 2016

use plain SQL dump or single migration with all db structure, and write migrations on release after development hell.

The problem with that is when you want to run fixtures and testing on your application specially when including CI.

@dynasource
Copy link
Member

I understand the reasoning, but this is way too complex. We should think from 1 dimension, otherwise you will get ambiguity issues. Its either a dependency mechanism or a m000000_000000 mechanism, not both.

@Faryshta
Copy link
Contributor Author

Faryshta commented Nov 1, 2016

@dynasource then lets think on another name for a non-linear tool to load the database schema.

Plus literally everything else in yii2 behaves non-lineary. Fixtures also deal with database data, and they load dependencies using hte Fixture::$depends property.

@dynasource
Copy link
Member

well, the thing is that the current mechanism is built around m000000_000000, which implies a chronology based on seconds. If you want to include other mechinisms, this will easily get out of hand.

To give you an analogy. In a perfect world, we would have a perfect version control system for databases. However, should Yii support such a advanced system? Should we create a GIT for DB's in Yii? We have to keep it simple. The more complex we make it, the more support we have to give.

@samdark
Copy link
Member

samdark commented Nov 1, 2016

The idea of dependencies for migrations isn't bad and actually makes sense but as @dynasource said, we can't have two concepts at the same time.

@Faryshta
Copy link
Contributor Author

Faryshta commented Nov 2, 2016

@dynasource i am not saying to replace the current mechanism, i understand its being used already. I am saying that we can build a second mechanism with another name (revisions, syncronizations, etc). We can use this issue to decide about naming and implementations.

@nkostadinov
Copy link

It will bring unnecessary complexity. It is already complicated enough with just a simple feature of namespaced migrations which dont work well with path based migrations.
In order to add dependency to migration you need to know what the migration does beforehand. There will be some unresolvable dependency cases, circular dependencies etc . It will not only bring dependency to migrations but also to the modules which contain that migration. Something like db composer - hell :)

@dynasource
Copy link
Member

@Faryshta, we all agree that having flexibility in the causality is great. We also agree on our fear to invest too many hours on this subject, ending up with our own migration framework (like composer, npm, git). I can also imagine that there are already third party packages available. However, if you are ambitious, we are not withholding you to create a separate yii extension for it of course.

@samdark
Copy link
Member

samdark commented Nov 2, 2016

Later this extension could replace current migrations if it will prove being good. I think this way is preferred since this is pretty new concept which needs to be proven.

@Faryshta
Copy link
Contributor Author

Faryshta commented Nov 2, 2016

instead of 'migration' which name should be used? I was thinking 'revision'.

@Faryshta
Copy link
Contributor Author

Faryshta commented Nov 2, 2016

or DbAsset

@dynasource
Copy link
Member

migration :)

@Faryshta
Copy link
Contributor Author

Faryshta commented Nov 2, 2016

wouldn't it cause confussion to have a timestamp migrations and asset migrations? i would prefer to call them differently, besides the asset migrations will use things such as namespaces to load dependencies.

@dynasource
Copy link
Member

it would be even more confusing if your extension about migrations does not contain the word migration. Unless your extension will get so popular and well known that it becomes a de facto standard. In that case you can call it anything you like ;)

Assets would not be preferable because its relates to files. Revisions relate to texts & articles, but is already better (covering a moment in time).

@samdark
Copy link
Member

samdark commented Nov 2, 2016

Calling these migrations is OK. All this stuff reminds me of how git works. If you haven't read Pro Git, I highly recommend it before thinking about architecture.

@dynasource
Copy link
Member

@samdark, this Pro Git book seems quite useful in general. Shouldnt we put this link somewhere in this document:

@samdark
Copy link
Member

samdark commented Nov 3, 2016

Won't hurt.

@samdark
Copy link
Member

samdark commented Nov 3, 2016

5ab5b40

samdark added a commit that referenced this issue Nov 3, 2016
@cebe cebe removed this from the 2.1.x milestone Nov 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants