silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS
https://www.silverstripe.org
BSD 3-Clause "New" or "Revised" License
719 stars 823 forks source link

RFC: Use Symfony Console Component for Tasks and better cli support #5542

Open axyr opened 8 years ago

axyr commented 8 years ago

Author: @axyr Status: Draft Version: 0.1

Introduction

Currently we can run Tasks or call (cli)Controllers on the command line with sake dev/build or calling php framework/cli-script.php dev/build.

While when we are on the command line, we can do much more to control and develop our website.

Symfony provides a very nice component to interact with our application on the command line.

http://symfony.com/doc/current/components/console/introduction.html

We can use the command line for much more then build tasks, and the Commands can be called from code as well.

We can still use dev/tasks from a browser to call the newly create TaskCommand and maintain backwards compatibility.

It becomes more easy to run Tasks/Commands from code and reuse pieces of functionality.

With commands we can create code scaffolders like make:page MyCustomPage, run cron tasks.

We can display more help information and options for complex tasks on the screen, instead of only buried in the documentation.

It will be easier for developers to write small maintenance or development commands.

Organizing code in Commands can clean up code and improve reusabilty.

Purpose and outcome

When writing tests, I find myself more time spending on the command line, in favor of refreshing my browser all the time. That makes me want having more tools on the command line that work and look good.

And since more and more people are familiar with git(hub) and composer for bootstrapping applications, the command line is more widely used and almost a requirement to start developing. Every module tells you to run composer require some/package.

The browser is only needed to flush and rebuild the cache (for now...?).

I did a lot of fiddling and find it very easy to extend and work with.

See for some examples. I use the Make Commands very extensive and it speeds up my boilerplating stage.

https://github.com/axyr/silverstripe-console

Now I will not say that all this is not possible with the current codebase of Silverstripe, but I think it can be done better.

Proposal

  1. All Task code will be moved to a Command.
  2. All Browser callable Tasks will call the newly created Command to preserve backwards compatibilty

    Alternatives

I could not find a better console package then Symfony provides.

Impact

Sake will call the Commands instead of Controllers. The syntax on the command line will change from :

sake dev/build flush=all to something like sake dev:build --flush=all

On the cli we can use enter sake to list all available commands. Do we want to have the same in the browser?

We might have Commands implement BrowserCallable to make them explicitly available for browser usage and not by default. We need to find out how to refresh caches for browser usage from the command line, since this is not always (never?) the case.

The formatting of messages like in DB::alteration needs to be refactored.

We might want to think of a nice color scheme for our info, error and warning messages in a blue-ish coloring scheme?

We can get rid of a lot of Directory::is_cli(); calls that are used for formatting and permission checks. A lot of that logic can be handled by the commands.

This is a nice way to reorganize some code when we gradually move pieces of functionality to Single Responsibility Classes that can be called by a command (and current use cases where the code is used) and in other places if we need them.

Our live will become easier and we gain more power and flexibility.

assertchris commented 8 years ago

I think it would be better for the RFC to remove the sake dev:build -f=a example from the RFC text. Shorthand options are a neat side-effect, but they may confuse the issue a little.

axyr commented 8 years ago

changed sake dev/build flush=all to something like sake dev:build -f=a or sake dev:build --flush=all to sake dev/build flush=all to something like sake dev:build --flush=all

:) thanks

sminnee commented 8 years ago

I like the idea of using the Symfony console tools. However I wonder if our work in this space should be done as a composer plugin that provides custom commands?

That way, users don't need to learn a new tool.

axyr commented 8 years ago

We could provide a wrapper to convert dev/tasks/MyTask option1=2&option2=3 to task:mytask --option1=2 --option3=4

So commandline and browser work the the same as it does now, with the option and power to use the new syntax.

Using the Symfony console syntax is more alike with 'normal' terminal commands that are also used for git and composer.

I think as soon as devs know they can use latter, a little cli experience is enough to recognize the syntax.

And people that are coming from symfony, yi, laravel or drupal, but want to use Silverstripe because they need a good CMS, will feel at home immediately, and don't need to learn a new tool?

sminnee commented 8 years ago

This is what I'm talking about: https://getcomposer.org/doc/articles/plugins.md#command-provider

Then you could support

Without needing a tool other than composer.

I agree on the need for a wrapper - otherwise people will be stuck with non-converted tasks and be forced to pick between 2 tools.

axyr commented 8 years ago

Ah that way.

I will inspect the composer providers a little bit more! I was not aware of that.

But it seems just a wrapper around Symfony Console.

It feels a bit confusing to type composer to interact with silverstripe instead of composer.

And it requires to 'namespace' all commands with ss?

axyr commented 8 years ago

And it will require composer on live servers.

stevie-mayhew commented 8 years ago

I don't like the thought of composer on live environments.

I am a big fan of the Symfony console component and think that it would be a great replacement for the sake command. It makes it a lot easier for us to input other commands into sake as modules as well which could be handy.

axyr commented 8 years ago

Me neither... it was an argument against it in addition to my previous comment ;)

dhensby commented 8 years ago

This is a fantastic idea that I'm sure many of us have thought about.

I'd like to see the framework CLI tools be a package/module that would easily allow the definition of new commands by modules.

What you've got so far in your module looks like excellent ground work!

tractorcow commented 8 years ago

I would be in favour of a native non-composer task runner built on Symfony. I've used it for cow and I find it an excellent framework.

You would need to find a solution to abstracting running of tasks via the web, however. Perhaps the symfony console component could be one interface, where the web is a separate one.

sminnee commented 7 years ago

The beta1 windows is closing. However if we retained the existing sake command for posterity and introduced this as a new tool, this can be done as a minor change.

robbieaverill commented 5 years ago

@silverstripe/core-team thoughts on making this a target for SS5 to replace sake and sspak?

There are two options available at the moment that could be used, merged, forked, or we could start fresh:

ScopeyNZ commented 5 years ago

I'm in favour. Right now I feel like the current task system is a little cumbersome especially as it uses HTTPRequest on CLI. Allowing developers to create "tasks" using Symfony console is a better experience.

sminnee commented 5 years ago

I'd probably say that tasks should be kept arms-length from both HTTPRequest and Symfony console.

These tasks could then be called from

tractorcow commented 5 years ago

Sounds good to me. Symfony based sake, but with symfony-agnostic based tasks.

You may want to wrap / abstract input / output interface for tasks though.

robbieaverill commented 5 years ago

It sounds like we're all in agreement to move forward with this. We probably need to agree on the input/output arguments for tasks though.

I'd be in favour of bringing in PSR-7 and using those interfaces, personally (https://github.com/silverstripe/silverstripe-framework/issues/6514).

sminnee commented 5 years ago

I'd be in favour of bringing in PSR-7 and using those interfaces, personally

But the core issue here is that tasks aren't necessarily HTTP, and CLI definitely isn't. How is moving from 1 HTTP interface to another HTTP interface helping anything?

For output, I think that PSR-3 (logging) is an appropriate interface to use. I don't think that task should be expected to provide more sophisticated or interactive output than a log.

For input, I'm not really sure. The input could simply be a map of properties, however, I'm not sure if we want some kind of structure for providing metadata about what properties are available.

sminnee commented 5 years ago

Also, it's quite likely that not all actions we might want to bolt into the CLI tool could be wrapped into this abstracted build tasks, but that most would be, and that where it's possible it has the benefit of running them in different context such as a job-queue, which is especially helpful for production environments and cronjobs.

tractorcow commented 5 years ago

We already integrate with a lot more symfony (whereas we used to do so with Zend). I wouldn't mind if we just used their IO abstraction directly for tasks. I have looked at their interfaces before, and I believe that they would well suit our use cases (e.g. cronjobs, manual tasks with CLI, other interactive / non-interactive environments).

I would look at something similar to queuedjobs interface, but with more care given to how tasks are persisted. :) E.g. use core wake / sleep methods instead of custom jobData(), implement serializable for storage, and a main entry point public function run(InputInterface, OutputInterface) to replace the old public function run($request)

We would also separate the concerns of the nature of the tasks themselves, versus how the tasks are created, invoked, or stored. That would be left up to third party modules, and we would simply provide a basic runner replacing sake.

robbieaverill commented 5 years ago

But the core issue here is that tasks aren't necessarily HTTP, and CLI definitely isn't. How is moving from 1 HTTP interface to another HTTP interface helping anything?

Yeah true, haha

+1 from me for using the Symfony IO interfaces.

I do feel like there's a place for a logger in tasks, but I'm not sure where I sit in its placement as the primary output interface

ScopeyNZ commented 5 years ago

I don't think we should use a log interface as output. Output should be a fair amount more dynamic than logging.

My vote is to use Symfony interfaces. Don't use any HTTP interface - we're making CLI tasks.

The only challenge is running tasks over HTTP. We can probably write something that takes __GET parameters and runs the command with --no-interaction and just forwards the output back into a streamed response?

Eg: dev/build?flush -> sake dev:build --flush dev/tasks/my/task?param=1&flag -> sake tasks:my:task --param="1" --flag

We could also redesign the output of these tasks when formatted as HTTP - I'm thinking some basic layout with a CLI-like themed <pre> where the content is streamed - similar to what we see on Travis builds.

robbieaverill commented 5 years ago

Further idea (from https://github.com/silverstripe/silverstripe-framework/issues/7992): have a default output showing task completed with the execution time. Note that this may come by default with symfony/console with the verbose flag, otherwise we could make it something that only shows with verbosity enabled.

sminnee commented 5 years ago

I don't think we should use a log interface as output. Output should be a fair amount more dynamic than logging.

I think that we're conflating two things here:

For pieces of code that are intended to be used in the CLI tool (whether it's called sake or something else), I agree.

However, build tasks are typically run in a variety of contexts:

Given this breadth of execution it's going to be important to clearly state what the limits of input and output are. My view is that PSR3 describes the intent of a build task's output very well: to provide a log of what happened and why.

If anyone disagrees with this, I would ask "what form of out output is inappropriately fed through a log that is also useable in the 4 different use-cases I've listed above?"

Output should be a fair amount more dynamic than logging.

I could see a use case for interactive / animated CLI tools with lots of colour (beyond the simple colour coding of error-levels with PSR3 would allow for). However, such code is unlikely to be very useful outside of CLI execution.

As such I think we should not use BuildTask or its replacement as the placeholder of such code, and think of that as something more like a ConsolePlugin.

robbieaverill commented 5 years ago

Ok fair enough! I think we need to review https://github.com/silverstripe/silverstripe-framework/issues/8044 as well, which would allow a task with a logger to use all of the PSR-3 levels without having them being chewed up in the SilverStripe core log handling

chillu commented 5 years ago

I'm in favour of this as well, but coming from another angle: There's no CSRF protection around web-based administrative commands. This can lead to DoS targeted at a single CMS author or admin user (not really DDoS). Depending on the dev tasks installed, it could also be destructive. Rather than adding CSRF protection, I'd propose that we focus our energies on making CLI usage great, deprecate calling dev/* and via web, and disable it by default in production mode (allowing devs to opt in for covering their edge cases).

Laravel allows you to define routes to call CLI commands through the web, but you have to opt-into that - which is a good balance I think.

I've collected related tickets:

sminnee commented 5 years ago

I'd propose that we focus our energies on making CLI usage great, deprecate calling dev/* and via web, and disable it by default in production mode (allowing devs to opt in for covering their edge cases).

Just as long as build tasks can still be enqueued as is currently the case if you install the queuedjobs module. This approach is an important use-case and should be our first recommendation for production servers where is SSH access is either unavailable or discouraged.

sminnee commented 5 years ago

To be clear: since queuedjobs 3.1, if you install queuedjobs, the behaviour of the dev/tasks web UI is replaced with one that provides links to enqueue jobs instead of running them directly.

sminnee commented 5 years ago

...this probably means that we should add CSRF protection.

chillu commented 5 years ago

...this probably means that we should add CSRF protection.

Discussed as SS-2018-022

robbieaverill commented 5 years ago

I'd love to see the dev/* routes gone from the browser by default at least. I understand the need for some support since not everyone has CLI access, so queued jobs seem like a good alternative to me. In a perfect world I think I'd rather see the management of that happening in the admin interface rather than in the dev/* space though

lerni commented 5 years ago

I like the separation of concerns of developer & content-author. Moving dev-tasks to the admin-interface goes against that and requires the admin-interface to be functional to run dev-tasks per HTTP. CLI for sure is the way to go but if it has to runs per HTTP, it has to be as simple as possible, which the admin interface isn't.

ScopeyNZ commented 5 years ago

I'd love to see the dev/* routes gone from the browser by default at least. I understand the need for some support since not everyone has CLI access, so queued jobs seem like a good alternative to me. In a perfect world I think I'd rather see the management of that happening in the admin interface rather than in the dev/* space though

If we're going down this path I think it makes a lot of sense to get the conversation started sooner rather than later. Some RFC to remove the HTTP interfaces for dev/build/ and dev/tasks (what about ?flush)? I don't really think I'm for this yet btw, I'd need some convincing.

dnsl48 commented 5 years ago

Symfony Console component becomes a Framework requirement

I don't see why it cannot live as a decoupled module. The only requirement would be for the framework to provide necessary APIs.

chillu commented 5 years ago

There's a simple "ansi to html" wrapper for showing coloured Symfony console output via a web browser: https://symfony.com/doc/current/console/command_in_controller.html