Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[MonologBridge] Reset loggers on workers#40761
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
34ad0d8 to3845558Compare
jderusse left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Would be great to provide the "glue" in monolog-bundle
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Bridge/Monolog/Messenger/ResetLoggersWorkerSubscriber.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
l-vo commentedApr 10, 2021
just pushed it:symfony/monolog-bundle#403 |
| CHANGELOG | ||
| ========= | ||
| 5.3.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| 5.3.0 | ||
| ----- | ||
| * Added`ResetLoggersWorkerSubscriber` to reset buffered logs in messenger workers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
missing empty line above
Added -> Add
fabpot commentedApr 13, 2021
Thank you@l-vo. |
005686c to1d2f7f1Comparegggeek commentedMay 11, 2021
Sorry for chiming in so late, just giving my 2c of advice: this is a nice feature to have, that is in fact useful forany long-running sf console script, be it a daemon or a huge batch script doing import/export of data and taking many hours to run. As such, it could probably use a better name than 'SomethingWorker', to make it more evident to developers that it is not tied exclusively to the Messenger component. |
l-vo commentedMay 11, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@gggeek It would be useful for any long-running process that uses a loop BUT this feature relies on a worker event. It's why it is named somethingWorker 🙂 |
gggeek commentedMay 11, 2021
@l-vo doh. I missed the namespace of the event, my bad. Wouldn't it be better then to have the worker emit instead a resetLogs event? |
l-vo commentedMay 11, 2021
@gggeek No it wouldn't. The reason is decoupling. The emitted event should not be aware of what the listeners are going to do. |
gggeek commentedMay 14, 2021
But then the monolog bridge is forced to know about this event - as well as any other event required by daemon-like scripts. It does not seem very different... |
This PR was merged into the 6.0 branch.Discussion----------[MonologBridge] Remove deprecated code| Q | A| ------------- | ---| Branch? | 6.0| Bug fix? | no| New feature? | no| Deprecations? | no| Tickets |#40761| License | MIT| Doc PR | N/ACommits-------53ce26e [MonologBridge] Remove deprecated code
Uh oh!
There was an error while loading.Please reload this page.
This PR tries to solve some problems with buffered handlers (FingerCrossed) in workers.
Let's consider the default configuration (
stop_buffering: true):buffer_size, it's a shame to keep logs of previous messages.Then with (
stop_buffering: false) (why isn't this the default configuration ?)stop_buffering: true, logs of previous messages are kept in memoryIn a similar way of
DoctrineClearEntityManagerWorkerSubscriber, this PR adds aResetLoggersWorkerSubscribberto reset resettable loggers.Integration in Monolog bundle:symfony/monolog-bundle#403