search

Kdyby / Console

Symfony Console integration for Kdyby components
https://packagist.org/packages/kdyby/console
Other
52 stars 54 forks source link

Shouldn't we run the Nette\Application\Application onShutdown event? #59

Closed Fuco1 closed 7 years ago

Fuco1 commented 8 years ago

onStartup is called because so far the control wasn't handled over to Kdyby\Console\Application, but the original onShutdown is ignored (you call onError however).

If this is undesirable, maybe a way to attach console-only shutdown event would be helpful.

My usecase right now is: I use symfony/process to start some background processes, but when the main process dies it kills the children (standard unix behaviour). So on the shutdown hook I want to wait for the child processes to finish, then really terminate. I want to wait up in both web request and console request, so for me a common handler is fine (plus you can always test there if you run in the cli)

fprochazka commented 8 years ago

Well, the default and simples way to use kdyby/console is through www/index.php, since in CLI it's proxied to symfony's Application, which means the Nette's Application is in control. If you look closely, the Kdyby's Application is not calling onStartup either, it's Nette.

The onShutdown is not called probably because of exit codes of the response. This might need to be rethinked.

About your use-case. IMHO it should not be implemented in the onShutdown, but in the logic of the command, since waiting for the children is part of the main job, isn't it?

Fuco1 commented 8 years ago

You're right about my problem, though we're dealing with such a mad architecture (legacy yay) this was the simplest way to get it going, which lead to discovering of this issue. Everything calls everything and we have quite literally no idea where the processes are forked/started ~_~.

Anyway, thanks for looking into this!

fprochazka commented 7 years ago

@Fuco1 Thank you for reporting this! If it still bothers you, can you please test the fix?

Fuco1 commented 7 years ago

@fprochazka Unfortunatelly I no longer work at the company so I can't test this :/ I'll notify my colleague so maybe they will take a look at it.

fprochazka commented 7 years ago

@Fuco1 Thank you!