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

improve migrate command #384

Closed
schmunk42 opened this issue May 23, 2013 · 47 comments
Closed

improve migrate command #384

schmunk42 opened this issue May 23, 2013 · 47 comments
Assignees
Milestone

Comments

@schmunk42
Copy link
Contributor

  • running migrations from different locations (modules)

more or less like this one from @cebe https://github.com/yiiext/migrate-command

@samdark
Copy link
Member

samdark commented May 23, 2013

Migrations from custom locations are already supported.

@qiangxue
Copy link
Member

I think the main enhancement here is about applying migration from modules:

  1. migrate/create xyz --module=admin: creates a new migration for "admin" module
  2. migrate/up: find new migrations from ALL modules and the main app, and apply them
  3. migrate/up --module=admin,app: only apply migrations from "admin" and "app"

I think this is worth doing. We mainly need to support array for $migrationPath to hold the migration path declarations for every module. "app" stands for the main app.

@samdark
Copy link
Member

samdark commented May 23, 2013

What about extensions?

@qiangxue
Copy link
Member

It doesn't have to be modules actually. It is essentially a collection of named migration paths. Perhaps we should rename the option.

The tricky part is how to "automatically register" a migration path. This is something we should think more. It is related with the process of installing an extension. This is easy if all extensions are installed via composer.

@cebe
Copy link
Member

cebe commented May 24, 2013

I'll handle this.

@schmunk42
Copy link
Contributor Author

I was able to configure the migrations in a bootstrap() method, like so:

        \Yii::$container->set(
            'yii\console\controllers\MigrateController',
            [
                'migrationLookup' => ['@app/migrations/my-sub-folder']
            ]
        );

But that would get overwritten by other modules doing the same, I guess.

Could we add an add() method to the DI container?
Or which instance could hold this information?

@qiangxue
Copy link
Member

I don't think DI container should support this.

There are two cases here:

  • Migrations for installed extensions: We probably can introduce a new element to Application::extensions which holds the migration path of an extension.
  • Migrations for internally developed modules: I'm not sure if this is really needed.

@schmunk42
Copy link
Contributor Author

What do you mean with internally developed modules, like subfolders in @app/migrations?

@qiangxue
Copy link
Member

I mean modules you develop within your application. These modules are not extensions.

@schmunk42
Copy link
Contributor Author

I'd like to be able to configure them how I need them.

Eg. I often use development-data migrations, which are only executed on the development systems and usually contain only data from the production server for testing/replication.

@qiangxue
Copy link
Member

Let's consolidate the discussion of this thread and #3273.

Here's how I envision it from the global picture:

  • We need a mechanism to allow different components (e.g. extensions, modules) to register a set of parameters (e.g. migration path, translation path) needed by a service (e.g. migration, translation).
  • We do not want to overuse bootstrap components because they will cause performance degradation on the critical path.
  • For extensions, we probably can introduce a params element that each extension can specify in its composer.json. The params element is an array specifying the needed parameters aforementioned.
  • For non-extension components, it seems to me developers have to manually enter the parameter information in Yii::$app->params. I'm not quite sure with this part.

@schmunk42
Copy link
Contributor Author

So having migrations and translation in extras sounds nice, not sure if it has to be wrapped in params:

"extra": {
    "bootstrap": "schmunk42\\playground\\Bootstrap",
    "migrations": ["@schmunk42/playground/migrations"]
    "messages": ["@schmunk42/playground/messages"]
}

This information should be written to @vendor/yiisoft/extensions.php.
Now it'd be nice to still have the possibility to add something to or modify Application::$extensions eg. for internal modules, maybe via a new property Application::extra and set the values in config/web.php:

"extra": [
    "migrations": ["@app/migrations/development","@app/migrations/MYSELF"]
    "messages": ["@app/testing/messages"]
]

Which would get merged with the values from extensions.php and available in the end with:

echo Yii::$app->extra['migrations'];

["@app/migrations/development", "@app/migrations/MYSELF", "@schmunk42/playground/migrations"]

@qiangxue
Copy link
Member

To avoid confusion, Application::$extensions should only be used by extensions. It should not be modified by internal modules or other non-extension components.

I think we should make it generic by using params. Otherwise, it would be impossible for other services to require similar "global" parameters. So it would be like this:

"extra": {
    "bootstrap": "schmunk42\\playground\\Bootstrap",
    "params": {
        "yii.migrations": ["@schmunk42/playground/migrations"],
        "yii.messages": ["@schmunk42/playground/messages"]
    }
}

These parameters can be automatically merged into Yii::$app->params. Notice that in the above, I'm using yii. prefix in the param naming to avoid potential conflict.

@schmunk42
Copy link
Contributor Author

Alright, but let's use yii2.migrations, OK?

Are there any other parts of the application which already rely on a specific params configuration - I am just curios.
Convention would be also to always merge the stuff from composer.json params into the application config, right?

@qiangxue
Copy link
Member

I think we should use yii.migrations, just like we use the namespace yii\db. These things are new to Yii. We don't need to worry about conflict with Yii 1.1.

For core components, right now we only need migration paths and translation paths. This feature opens the possibility for non-core extensions to require similar kind of global parameters.

Yes, params in composer.json will be merged into app config's params so that a global parameter can always be accessed via Yii::$app->params[$paramName], where $paramName is named by convention.

@yiisoft/core-developers any comments on this feature?

@philippfrenzel
Copy link
Contributor

hi, as I don't have an opinion on how to implement - after you decided, can u pls. make an short example on how to use it, as this is what I'm currently looking forward too;) Thanks!

@samdark
Copy link
Member

samdark commented Apr 29, 2014

@qiangxue I'm confused. Are we talking about migrations or general way to allow components to affect overall application behavior?

@schmunk42
Copy link
Contributor Author

@samdark The discussion emerged from the migrations feature, since we need a way to register migrations (and other stuff) in the application.

The migrate command discussed here would make use of the command line param --migrationPath and config param yii.migrations.

@qiangxue
Copy link
Member

@samdark We are talking about a general way of letting a service to gather information that could be provided by different components.

Migration and translation are two most obvious examples. For migration, the command needs to gather a list of migration paths that could be specified by installed extensions or modules developed within an app. For translation, I18N needs to gather translations from different extensions and modules.

We do not want to place these extensions or modules in the bootstrapping process for performance reason. So I'm suggesting to use params to pass along the information. See #384 (comment)

@samdark
Copy link
Member

samdark commented Apr 29, 2014

You mean extension will write something to params on installing it?

@cebe cebe removed this from the 2.0.3 milestone Feb 23, 2015
@vladim1
Copy link

vladim1 commented Mar 29, 2015

I'll leave this post here to not create the same one.
It's really pain in the ass to run migrations of the plugged in modules with specifiing their pathes.
This issue is almost 2 years. I am waiting for this feature from milestone to milestone. What is the problem with solving it? So, we could discuss it...
PS: Is there any tool/extension to automate the process of running migrations of plugged in modules?

@schmunk42
Copy link
Contributor Author

@Faryshta
Copy link
Contributor

I think #8202 is a good way to fix this issue for modules, extensions, external libraries and all the other cases discused here.

@jacmoe
Copy link

jacmoe commented Feb 6, 2016

Since I include migrations in my test routines - ie migrations should be able to migrate all the way down and all the way up at any time - I am looking forward to a built-in solution. 👍
The https://github.com/dmstr/yii2-migrate-command is part of my standard tool-kit already, but it would make Yii even more cool if it was built in. Especially since newcomers wouldn't have to discover it themselves. :)

@klimov-paul
Copy link
Member

I whould solve this matter using namespaces instead of paths, just I have proposed here:
#9698 (comment)

@schmunk42
Copy link
Contributor Author

@klimov-paul We're currently using aliases, which is essentially the same, isn't it? #384 (comment)

@ItsReddi
Copy link
Contributor

ItsReddi commented Aug 9, 2016

I discovered this issue before two years. Well the eta was RC. Any chance that this feature will be implemented in the next release or should we stick to some extensions that can handle this approach?

@klimov-paul
Copy link
Member

Solution proposed at #12511

@klimov-paul klimov-paul modified the milestones: 2.0.10, 2.0.x Sep 12, 2016
@klimov-paul
Copy link
Member

Resolved by commit 8aa0e85

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

Successfully merging a pull request may close this issue.