[WIP][Console] Add ContainerAwareCommand #10087
Conversation
| * @return ContainerInterface | ||
| */ | ||
| protected function getContainer() | ||
| { | ||
| if (null === $this->container) { | ||
| if (null === parent::getContainer()) { | ||
| $this->container = $this->getApplication()->getKernel()->getContainer(); |
There was a problem hiding this comment.
You have to either use setContainer or make $container protected
There was a problem hiding this comment.
Thanks, overlooked that one. I'll wait for the feedback if we still need this piece of code before fixing it.
|
Do you have an example of application using both the I would give advantage to a proper dependency injection instead of promoting |
|
Motivation is our in house application that is not open source. Until there's a way to lazy load services for commands we're not ready to go DIC for them as we have some frequently running cronjobs using the console. Another application would be phpBB, but as far as I can tell they're using dependency injection for their commands. I'd agree that this is probably a rare set up, so this is probably rather unattractive if ContainerAwareCommand can't be removed completely from FrameworkBundle. |
|
I don't see the benefit of adding this class in the Console component: the container won't be injected in the command unless you use an Application implementation supporting it, which is not in the component either. and the class in FrameworkBundle cannot be removed for BC reasons |
|
I should have worded that last part more carefully. We obviously need to retain the class in the FrameworkBundle for 2.x. I meant if we can remove all the code in it so we can eventually remove it. |
|
I did my homework, there are a few open source projects that have a copy of this code in their repository: |
|
I also fail to see the benefit of such a class in the Console component as the most important code to make it work transparently is still in the framework bundle: https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/Console/Application.php#L80-L84. |
| /** | ||
| * Command. | ||
| * | ||
| * @author Fabien Potencier <fabien@symfony.com> |
There was a problem hiding this comment.
he is not the author, or add yourself too right? 👶
There was a problem hiding this comment.
Well since it's essentially a copy of Symfony\Bundle\FrameworkBundle\Command\ContainerAwareCommand plus a small modification to getContainer so that only seemed fair.
|
@fabpot I understand where you're coming from, as this is "some assembly required". As mentioned above, this only makes sense if we can remove this condition: https://github.com/realityking/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/Command/ContainerAwareCommand.php#L35 I think it's safe, but I'd like someone with more knowledge of the ins and outs to confirm that. |
|
@realityking The code you mention can be safely removed, but again, you would still need the code from the FrameworkBundle Application class to make this work "out of the box". |
|
@GromNaN I do have some CLI projects which use the DIC, however I've never injected the container in my commands... |
|
@gnugat I'm using a lot the Console standalone or with Pimple and I regret that so many open-source Commands are tied to the Symfony/DI component. |
|
@GromNaN nobody needs the The Console's default Application doesn't inject the container to commands which implement this interface. @realityking If you want to inject the container into your commands, you don't need |
|
Closing now. |
In the spirit of #7435 I propose adding a ContainerAwareCommand to the Console Component. This is helpful when implementing both the DIC and the Console.
I'm fairly certain the code remaining in ContainerAwareCommand in the FrameworkBundle could also be dropped as the Container is injected into every command by the Application. This would also make it a little easier to move code between Projects using the Framework and those using only the Components.
Todo: