Flush cache manager at command termination (fix #32) #38
Conversation
| */ | ||
| public function onConsoleTerminate(ConsoleTerminateEvent $event) | ||
| { | ||
| $this->cacheManager->flush(); |
There was a problem hiding this comment.
i was wondering if we could return the number of requests we flushed in the CacheManager and use that to output a line on the console "Sent 5 invalidation requests to the caching proxy." - for the verbose mode.
There was a problem hiding this comment.
This can be done, with some changes in the library. And there’s some ambiguity: does “5 requests” mean: 5 requests to each proxy server, or 5 requests in total?
There was a problem hiding this comment.
i would mean to each proxy. making this more explicit makes sense yes. btw, i would only ouptut a message when there are > 0 requests, as otherwise we would output that line after every command run.
and yes, we would need to change on the library, but i think it would make sense. shall i do a PR on the library?
There was a problem hiding this comment.
|
thanks. made some minor comments. the only important thing is the question how we behave in error situations. |
| * | ||
| * @param GetResponseForExceptionEvent $event | ||
| */ | ||
| public function onKernelException(GetResponseForExceptionEvent $event) |
There was a problem hiding this comment.
given that you don't use the event, the argument is useless (and so you could have a single method)
|
what about the terminate/exception situation and stofs input to reduce the number of methods? |
|
Think the terminate/exception has been cleared up as far as possible. I separated the console event methods because we want to log in those cases. |
|
cool, looks good to me. please merge and squash. |
Flush cache manager on kernel and command exception, too Revert minimum-stability to dev Reduce number of terminate methods
Improve PHPDocs
Flush cache manager at command termination (fix #32)
No description provided.