0

My question here boils down to whether what I'm doing in and of itself is wrong or whether I have discovered a bug. I'm simply not sure...

What did I do? We have a relatively normal application based on Symfony 2.8. There, we also use the usual service container. Also, for maintenance, we define a bunch of commands to call on the commandline. During development, I added another parameter to the constructor of a class which is used in one of these commands.

The problem I have now is that I can't properly deploy this. When I just replace the code and call app/console cache:clear to regenerate the cached service container from the services definition, because on startup, the kernel tries to instantiate all commands using the cached service container. Since the cached service doesn't provide the new parameter but the changed code requires the parameter, startup fails.

Further observations:

  • The command extends the setContainer() method from the baseclass to retrieve its own dependencies. In this case, one commands fails to retrieve its dependencies from the service container. I'm wondering if setContainer() is too early -- If I postponed this to execute(), this would only fail when the command is actually invoked but not when cache:clear is invoked.
  • I'm also thinking that Symfony shouldn't abort startup if initialization of some command fails. It should alert the user but continue loading other commands. Even more, it shouldn't even load commands that aren't used.
  • In my particular case, I think that just not loading that command (or skipping it on error) would do the job, but I'm not sure if that is always the case.
  • As workaround, I can of course delete the cache folder. But what's the point of a cache:clear command if that doesn't work?
Ulrich Eckhardt
  • 662
  • 6
  • 14
  • Back in the good ole days (S2.0/2.1) it was not uncommon to have to remove the cache directory manually. Gradually, most of the issues for doing so have been solved. Sound like you stumbled on another one. Extending setContainer is never a good thing. If your command needs dependencies then either inject them or pull them when needed. – Cerad Jul 22 '16 at 14:45
  • Thanks for the feedback, @Cerad! Since we're using the classname-based approach, we don't define any dependencies e.g. in the `services.yml`, so the best we can is to retrieve the dependencies on demand, right? I'll try converting the code to explicit registration of the commands via `services.yml` though, which seems to be the clearer approach anyway. I could imagine that that doesn't work either though. – Ulrich Eckhardt Jul 25 '16 at 07:44
  • For the record, defining the commands in the services and injecting dependencies doesn't work. Regardless of the called command, all commands are instantiated. I'd report this as a bug, or? That said, I tried postponing the call to `setContainer()` (I couldn't find any indication that extending it is somehow bad, btw) to the point where the command is actually called and was able to make things work. Apart from unbreaking some code, this also seems like a useful performance improvement, because nothing is as fast as avoiding unnecessary tasks. What do you think? – Ulrich Eckhardt Jul 25 '16 at 15:54
  • All commands are always instantiated by design. I suppose you could suggest an alternative but you would have to figure out how the console can access the configure section of a command without an instance. Maybe some sort of caching? I'm glad you found something that works. To be honest, I have no idea what it is that you are trying to do. In my world, you would either define a command as a service and inject the dependencies or define the command in the Command directory and pull the dependencies from the container as needed. You appear to have a third approach. – Cerad Jul 25 '16 at 16:12
  • Not really, I'm just mixing the two. The side-effect of pulling the dependencies as needed is that they are indeed only pulled on demand. The consequence of extending `setContainer()` or injecting them, is that they are always pulled. That means that if the dependencies for a single command can't be satisfied, then none of the other commands can be used either. If this is caused by an outdated service container cache, you can't even call the command required to clear that cache. I'll try to extract a minimal example and report an according bug. – Ulrich Eckhardt Jul 25 '16 at 17:44
  • BTW: The changes I tried work as follows: During startup, the code first registers all commands and then iterates over them, calling `setContainer()` on those derived from `ContainerAwareInterface`. I replaced that by just registering the commands. Lateron, when actually calling the selected command, I call `setContainer()` only on that single command, which circumvents issues with the dependencies of other commands. Calling `configure()` is unaffected by that, because it is done before calling `setContainer()` and it is explicitly documented so. – Ulrich Eckhardt Jul 25 '16 at 17:51
  • Issue reported here: https://github.com/symfony/symfony/issues/19429 – Ulrich Eckhardt Jul 26 '16 at 08:23

0 Answers0