-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Lock] Log already-locked errors as "notice" instead of "warning" #26020
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Lock] Log already-locked errors as "notice" instead of "warning" #26020
Conversation
Travis failure are not related and seems to be already adressed in another PR. |
What's about injecting a logger with a dedicated channel instead of adding another argument to the Lock constructor? |
@jderusse but log channel != log level, isn't it? |
Hum, actually, shouldn't we just change these "warning" to "notice"? A lock failure is normal situation for a lock system. "notice" better fits the description of the log level IMHO. |
Yep, it's not the same thing. But I wonder if the purpose of this PR wasn't just to reduce noise. That's why I suggested to use channel and move logs to another stream (this could be configured by the user without modifications on SF). I'm also fine with reducing log level (I don't remember why I chose warning at first), Is it considered as BC break? Anyway I agreed that configuring log level is not the right way to do. |
ping @omnilight can you describe your use case? IMHO we should just turn that "warning" into "notice" in 3.4, as bug fix. |
Our usecase: we use lock component for preventing console command run when
other instance is running. This is not a warning at all
|
ok, so @Simperfit let's turn "warning" into "notice" on 3.4 and done IMHO |
For us it will be ok
4 февр. 2018 г. 17:56 пользователь "Nicolas Grekas" <
notifications@github.com> написал:
… ok, so @Simperfit <https://github.com/simperfit> let's turn "warning"
into "notice" on 3.4 and done IMHO
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#26020 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADJ0KdpLYmPfNLx3jFLatOQNsobsgxXXks5tRcUegaJpZM4R3IGw>
.
|
1c64a3d
to
d930cd8
Compare
01520a7
to
cee77a9
Compare
done @nicolas-grekas |
cee77a9
to
52222b4
Compare
52222b4
to
2a74edb
Compare
Thank you @Simperfit. |
…arning" (Simperfit) This PR was merged into the 3.4 branch. Discussion ---------- [Lock] Log already-locked errors as "notice" instead of "warning" | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes <!-- don't forget to update src/**/CHANGELOG.md files --> | BC breaks? | no | Deprecations? | no <!-- don't forget to update UPGRADE-*.md files --> | Tests pass? | yes | Fixed tickets | #25887 <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | todo We add the ability to specify the log level we want in the lock component. That could be useful when parsing the logs.  Commits ------- 2a74edb [Lock] Log already-locked errors as "notice" instead of "warning"
We add the ability to specify the log level we want in the lock component. That could be useful when parsing the logs.