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

Console commands run #1764

Closed
Ragazzo opened this issue Jan 4, 2014 · 53 comments
Closed

Console commands run #1764

Ragazzo opened this issue Jan 4, 2014 · 53 comments
Assignees
Labels
type:docs Documentation
Milestone

Comments

@Ragazzo
Copy link
Contributor

Ragazzo commented Jan 4, 2014

Would be great to have ability to run console commands inside each other if user want to. Example: user designed command and want it to be able also to apply migrations (and then make some decisions on exit-code results), maybe call some other his written command. In Yii1 i was needed in such case, but there is nothing about it for Yii1. I got brief look of Yii2 console commands handling, should not be difficult. But anyway would be great if someone from core developers will handle this one. Proposed syntax

/**
 *
 * @param array|string $command command name to run. If array then first element
 * is command name, other are command options. ['migrate/mark', 'params' =>[20],
 * 'options' => ['db' => 'myConnection']]
 * @param boolean $isolated if command should run in isolated env. (application). 
 * New application will be created if needed.
 *
 */
public function call($command,$isolated)
@qiangxue
Copy link
Member

qiangxue commented Jan 4, 2014

Isn't Controller::run($route, $params = []) the one for this?

@Ragazzo
Copy link
Contributor Author

Ragazzo commented Jan 4, 2014

@qiangxue yep, but i would like to have handful method, because for those developers that are not into core, it could be a little be difficult to guess for that. So useful method that can be a wrapper is just the case.
But dont know how isolation will be handled in this case as it would be simple wrapper against Controller.

@qiangxue
Copy link
Member

qiangxue commented Jan 4, 2014

The run() method is designed to be used by users. Why call() is more understandable than run()?

@Ragazzo
Copy link
Contributor Author

Ragazzo commented Jan 4, 2014

Why call() is more understandable than run()?

yes. And currently how would i call my migration command for example migrate/mark 20 --db='my_connection' with Controller::run($route, $params = []) could you give an example?

@qiangxue
Copy link
Member

qiangxue commented Jan 4, 2014

run('migrate/mark', ['db' => 'my_connection', 20])

@Ragazzo
Copy link
Contributor Author

Ragazzo commented Jan 4, 2014

hm, so actions parameters should only be after controller properties? for example

run('migrate/mark', ['db' => 'my_connection' , 'table' => 'some_my_table', 20])

and not

run('migrate/mark', ['db' => 'my_connection', 20, 'table' => 'some_my_table'])

Anyway call() method would be very good for end user, because he is not good with internals. And in simple controller-run we can not make isolated call.

@qiangxue
Copy link
Member

qiangxue commented Jan 4, 2014

No, you can put action parameters before options or mix them.

I still don't understand the difference between call() and run() other than the extra $isolated parameter.

@qiangxue
Copy link
Member

qiangxue commented Jan 4, 2014

Also note that if you want to create new application instance, the old one will be gone because of the singleton.

-- update: actually, the one app is still there, but the new one will take Yii::$app. And you may end up using the new app by the old command after the new command finishes.

@Ragazzo
Copy link
Contributor Author

Ragazzo commented Jan 4, 2014

I still don't understand the difference between call() and run() other than the extra $isolated parameter.

for end user it is simpler. Also one call() against (new Controller)->run(). And yes second param. User can even override cal() method if needed. So definitely call

Also note that if you want to create new application instance, the old one will be gone because of the singleton.

yes i know, very bad practice to be true, dont know why it was accepted. Anyway we can save Yii::$app before creating new one, similar to $oldController when application run.

@qiangxue
Copy link
Member

qiangxue commented Jan 4, 2014

How will the new application instance be created?

@Ragazzo
Copy link
Contributor Author

Ragazzo commented Jan 4, 2014

i was thinking about $config (and if class is set there than creating application with that class) param as 3rd param for method call or we can simply do

new Application(require(Yii::getAlias('@app/console/config.php')))

or just require yii file under the framework @app path, but need to set some $_SERVER['argv'] then.

@qiangxue
Copy link
Member

qiangxue commented Jan 4, 2014

I think we should still use run() method because introducing another similar method call() would cause confusion.

We can introduce an extra parameter $isolated to run().

Application needs to save its initial configuration array internally and provides a method to re-create itself using the saved configuration.

@qiangxue
Copy link
Member

qiangxue commented Jan 4, 2014

Note that this method is not just used by console controllers, it can also be used by web controllers.

@Ragazzo
Copy link
Contributor Author

Ragazzo commented Jan 4, 2014

Note that this method is not just used by console controllers, it can also be used by web controllers.

yes, thats why we should add call to yii\console\Controller and not to yii\base\Controller to not touch web-controllers.

We can introduce an extra parameter $isolated to run().

same as said above. But this could be used in HMVC for example in Yii1 it was like forward(). But if for HMVC we need also to save $_POST/$_GET if needed (child should not get params of parent.).

Application needs to save its initial configuration array internally and provides a method to re-create itself using the saved configuration.

not sure if it is needed since it is singleton, maybe then just remove Yii::$app set in constructor so it will not be singleton? or yes some getConfig() getter would be nice.

@qiangxue
Copy link
Member

qiangxue commented Jan 4, 2014

...I still don't get why you insist in adding call(). What's the difference between call() and run() if they both have $isolated?

not sure if it is needed since it is singleton...maybe then just remove Yii::$app set in constructor so it will not be singleton?

We still need the singleton. Otherwise how will you access application components?

new Application(require(Yii::getAlias('@app/console/config.php')))

This is not good. Internal code should not need to know how the configuration is obtained. The configuration could be coming from some cache. That's why I suggested saving the configuration within the application so that it could create a new one using it. The drawback is that it requires extra memory.

@Ragazzo
Copy link
Contributor Author

Ragazzo commented Jan 4, 2014

...I still don't get why you insist in adding call(). What's the difference between call() and run() if they both have $isolated?

only because for console it makes more sense. also call is simle wrapper against Controller:run(). Or you sugges for end user to do this (new yii\console\Controller)->run('migrate/mark',[20]) when he wants to call custom command? I sure he will create then this method-wrapper by himself in 100%. So it is better to have it. And as for controller i dont think that Controller::run() should have $isolated because this param only makes sense for calling commands from commands and not for regular usage, so it is more of internal call logic then by Controller:run.

We still need the singleton. Otherwise how will you access application components?

like in Yii1, dont see any changes here. Yii::setApplication($app) vs Yii::$app. Our benefit is to eliminate strange unexpected behavior as setting in constructor Yii::$app.

This is not good. Internal code should not need to know how the configuration is obtained. The configuration could be coming from some cache. That's why I suggested saving the configuration within the application so that it could create a new one using it. The drawback is that it requires extra memory.

true, memory question i think can be solved by application property public $saveConfiguration = false; by default. In console config it can be true.

@qiangxue
Copy link
Member

qiangxue commented Jan 4, 2014

No, it's as simple as $this->run('migrate/mark', ['db' => 'my_connection', 20]);.

@Ragazzo
Copy link
Contributor Author

Ragazzo commented Jan 4, 2014

But we dont need $isolated in run().

@qiangxue
Copy link
Member

qiangxue commented Jan 4, 2014

Why? This is just a parameter controlling how an action should be run. Supposedly, this is also useful for implementing HMVC. That is, it may be needed by web controllers.

@Ragazzo
Copy link
Contributor Author

Ragazzo commented Jan 4, 2014

Because if you want full HMVC then you need to isolate other input parameters as GET/POST i dont think it would be added in next few days. But yes your argumentation is valid.

@qiangxue
Copy link
Member

qiangxue commented Jan 4, 2014

True, there's more work to do to fully support HMVC, which is not in our near-term plan.

@Ragazzo
Copy link
Contributor Author

Ragazzo commented Jan 4, 2014

Yes, thats why i think to not confuse users we can add simple wrapper call and then after release if needed add hmvc support.

@qiangxue
Copy link
Member

qiangxue commented Jan 4, 2014

My point is that the name call() and run() are so similar and their functions are also very similar. It will cause more confusion. By adding $isolated parameter to run(), we can write clearly in the API comment about what isolation it means. Note that even for console command, we don't achieve full isolation because of $_SERVER.

@Ragazzo
Copy link
Contributor Author

Ragazzo commented Jan 4, 2014

My point is that the name call() and run() are so similar and their functions are also very similar.

nope, if we will not add HMVC now. Then call will have some code for handling application creating with $isolated param, and run will not.

Note that even for console command, we don't achieve full isolation because of $_SERVER.

can back-up it, i guess. yii\console\Application::_backUp after application is executed can restore. Just an idea, we can find better solution i think, or we can avoid it by setting params explicitly.

@klimov-paul
Copy link
Member

From my point of view this discussion is pointless.

Example: user designed command and want it to be able also to apply migrations (and then make some decisions on exit-code results)

I can see only one appropriate solution for this: extract the migration functionality into the separated class (maybe application component), while the “MigrateCommand” should be only an interface wrapper around it. This will allow developer to reuse the migration mechanism in any place and any interface he wants.

Because if you want full HMVC then you need to isolate other input parameters as GET/POST

I am afraid it is unreachable with the current framework architecture: just see “Pagination::getPage()” for an instance. If some listing should be a part of “slave” controller, how we should handle such things?

Introducing a new method “call” does not make any sense. There is “run()” method already, it usage is not so complex as you think, while it give developer more clear understanding of the process.

@Ragazzo
Copy link
Contributor Author

Ragazzo commented Jan 4, 2014

There is “run()” method already, it usage is not so complex as you think, while it give developer more clear understanding of the process.

not sure about it, and once again there was discussion also about $isolated param to. So for end-user it is not so simple and obvious to use run. I think that almost all users before this issues was created did not know that they can run sub-commands with controller-run.

@samdark
Copy link
Member

samdark commented Jan 4, 2014

Noone actually asked :)

@Ragazzo
Copy link
Contributor Author

Ragazzo commented Jan 4, 2014

Yes, nobody actually knew about it i think.

@klimov-paul
Copy link
Member

My opinion: call one controller from another is a bad practice of code reusage. It breaks the meaning of MVC. If you wish some piece of code to be used in 2 different controllers, then extract it into separated class: component, helper, model, whatever.
I do not think framework should promote such practice, introducing special method for it.

there was discussion also about $isolated param

I don’t get it. We have “request” and “response” components already. Their usage ensures the particular controller returns its result as “Response” instance, while request parameters are fetched from “Request” instance. Console controller has a property “interactive”, which determines if it would try to fetch user input or not.
What else are you planning to isolate?
At the moment, this parameter just sounds like some magic trigger, which will suddenly resolve all possible problems with nested controller usage, while those problems should be avoided by wise application design (see above).

@Ragazzo
Copy link
Contributor Author

Ragazzo commented Jan 5, 2014

It breaks the meaning of MVC. If you wish some piece of code to be used in 2 different controllers, then extract it into separated class: component, helper, model, whatever.

yes, i wish some piece of code to be used in 2 different controllers and yes as an example it already extract it into separated class, for example migration command. But now i need to call it via $this->run(). This is not good, for reasons mentioned above.

What else are you planning to isolate?

Environment in which new command will be executed.

@Ragazzo
Copy link
Contributor Author

Ragazzo commented Jan 5, 2014

@klimov-paul i understand, but once again your use-case is specific, you only described drawbacks and no features. I said about some features, dont know why i am fighting to be true) In L4 you can easily call another command from command, if Yii architecture is not good enough for that, then what is the question? :)

@klimov-paul
Copy link
Member

you call $this->call('migrate/apply') and it will be executed

What is wrong with Yii::$app->runAction('migrate/apply')?

@Ragazzo
Copy link
Contributor Author

Ragazzo commented Jan 5, 2014

name is wrong, if we are not talking about isolation.

@klimov-paul
Copy link
Member

name is wrong

Why?

@klimov-paul
Copy link
Member

Method Module::runAction() accept route and action params, while returning Response instance. It should be just what you need.

@Ragazzo
Copy link
Contributor Author

Ragazzo commented Jan 5, 2014

because end user is calling command and not running action. it is more intuitive to do $this->call() then $this->run()

@Ragazzo
Copy link
Contributor Author

Ragazzo commented Jan 5, 2014

Method Module::runAction() accept route and action params, while returning Response instance. It should be just what you need.

nope, @qiangxue answered above about run() method.

@adamaltman
Copy link
Contributor

http://www.google.com/trends/explore#q=run%20command%2C%20call%20command&cmpt=q

run command is more popular than call command.

@Ragazzo
Copy link
Contributor Author

Ragazzo commented Jan 8, 2014

@adamaltman its not only about the name :D

@qiangxue qiangxue added this to the 2.0 RC milestone Apr 16, 2014
@samdark
Copy link
Member

samdark commented May 7, 2014

$oldApp = \Yii::$app;
new \yii\console\Application([
    'id' => 'Command runner',
    'basePath' => '@app',
    'components' => [
        'db' => $oldApp->db,
    ],
);
\Yii::$app->runAction('migrate/up', ['migrationPath' => '@yii/rbac/migrations/', 'interactive' => false]);
\Yii:$app = $oldApp;

@qiangxue can it be done simpler?

@samdark samdark self-assigned this May 7, 2014
@qiangxue
Copy link
Member

qiangxue commented May 7, 2014

Do you mean running a console command from a Web application?

@samdark
Copy link
Member

samdark commented May 7, 2014

From a web application or tests or another console application or whatever.

@qiangxue
Copy link
Member

qiangxue commented May 7, 2014

What about using system() or exec()?

@samdark
Copy link
Member

samdark commented May 7, 2014

Many hosts won't allow you using it.

@samdark
Copy link
Member

samdark commented May 27, 2014

@qiangxue so should we introduce shortcut method for it?

@Ragazzo
Copy link
Contributor Author

Ragazzo commented May 27, 2014

i am fine with Yii::$app->runAction(), not sure for isolation level, i think this one has low priority )

@samdark samdark modified the milestones: 2.0 GA, 2.0 RC May 31, 2014
@Ragazzo Ragazzo closed this as completed Jun 27, 2014
@cebe cebe modified the milestones: 2.0 RC, 2.0 GA Jun 29, 2014
@stepanselyuk
Copy link

Hi guys!
So how to do it now? (Run specified console command from a web application or tests or another console application or whatever.)

I tried as suggested Samdark, but it's not worked:

class ConsoleAppHelper
{

    public static function runAction( $route, $params = [ ], $controllerNamespace = null )
    {

        $oldApp = \Yii::$app;

        // fcgi doesn't have STDIN and STDOUT defined by default
        defined( 'STDIN' ) or define( 'STDIN', fopen( 'php://stdin', 'r' ) );
        defined( 'STDOUT' ) or define( 'STDOUT', fopen( 'php://stdout', 'w' ) );

        /** @noinspection PhpIncludeInspection */
        $config = require( \Yii::getAlias( '@app/config/console.php' ) );

        $consoleApp = new ConsoleApplication( $config );

        if (!is_null( $controllerNamespace )) {
            $consoleApp->controllerNamespace = $controllerNamespace;
        }

        // use current connection to DB
        \Yii::$app->db = $oldApp->db;

        $exitCode = \Yii::$app->runAction( $route, array_merge( $params, [ 'interactive' => false ] ) );

        \Yii::$app = $oldApp;

        return $exitCode;
    }
} 

I didn't see results (it should be generate file in runtime path) of run action.

@stepanselyuk
Copy link

I've fixed some errors in the code above, and it work now, but I can't specify parameters to the action.

/**
 * Class ConsoleAppHelper
 * @package app\components
 */
class ConsoleAppHelper
{

    public static function runAction( $route, $params = [ ], $controllerNamespace = null )
    {

        $oldApp = \Yii::$app;

        // fcgi doesn't have STDIN and STDOUT defined by default
        defined( 'STDIN' ) or define( 'STDIN', fopen( 'php://stdin', 'r' ) );
        defined( 'STDOUT' ) or define( 'STDOUT', fopen( 'php://stdout', 'w' ) );

        /** @noinspection PhpIncludeInspection */
        $config = require( \Yii::getAlias( '@app/config/console.php' ) );

        $consoleApp = new ConsoleApplication( $config );

        if (!is_null( $controllerNamespace )) {
            $consoleApp->controllerNamespace = $controllerNamespace;
        }

        try {

            // use current connection to DB
            \Yii::$app->set( 'db', $oldApp->db );

            ob_start();

            $exitCode = $consoleApp->runAction(
                $route,
                array_merge( $params, [ 'interactive' => false ] )
            );

            $result = ob_get_clean();

            \Yii::trace( $result, 'console' );

        } catch ( \Exception $e ) {

            \Yii::warning( $e->getMessage() . ' in ' . $e->getFile() . ' on line ' . $e->getLine(), 'console' );

            $exitCode = 1;
        }

        \Yii::$app = $oldApp;

        return $exitCode;
    }
} 

@stepanselyuk
Copy link

Upd:
I've changed line

$exitCode = $consoleApp->runAction(
                $route,
                array_merge( $params, [ 'interactive' => false ] )
            );

to

$exitCode = $consoleApp->runAction(
                $route,
                array_merge( func_get_arg( 1 ), [ 'interactive' => false, 'color' => false ] )
            );

since it is collision with variable used in console.php (config from base application), or can use other variable name in the function parameters. Now all working. For future independent behavior (of console.php variables) we can change $route to func_get_arg( 0 ), and $controllerNamespace to func_get_arg( 2 ).

@cebe
Copy link
Member

cebe commented Dec 1, 2014

@mahsa92 please use the forum to ask questions. Or open a new issue to request a feature or report a bug.

@yiisoft yiisoft locked and limited conversation to collaborators Dec 1, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type:docs Documentation
Projects
None yet
Development

No branches or pull requests

7 participants