DM-17874: Remove ContextLog class and related code #40
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit is just to test that dependencies still work fine. There will be more changes to simplify Log class and update documentation and examples.
timj
reviewed
Feb 14, 2019
src/Log.cc
Outdated
| * Return a logger which is a descendant to this logger. | ||
| * | ||
| * If for example name if this logger is "main.task" and suffix is | ||
| * "subtask1.algorithm" then this method will return logger with tthe name |
timj
reviewed
Feb 14, 2019
src/Log.cc
Outdated
| /** | ||
| * Return a logger which is a descendant to this logger. | ||
| * | ||
| * If for example name if this logger is "main.task" and suffix is |
andy-slac
force-pushed
the
tickets/DM-17874
branch
from
ec83c98
to
b4f1ff9
Compare
timj
approved these changes
Feb 14, 2019
|
@timj, thanks! I'll try to find someone to look at mutex stuff. Always worth to have someone else checking multi-threading code 👀 |
ktlim
approved these changes
Feb 15, 2019
src/Log.cc
Outdated
|
|
||
| if (! skipInit) { | ||
| std::lock_guard<std::mutex> lock(configMutex); | ||
| if (! initialized) { |
There was a problem hiding this comment.
https://developer.lsst.io/cpp/style.html#the-following-white-space-conventions-should-be-followed says "No spaces separating unary operators and their arguments." (The old code was wrong.)
| @@ -110,13 +109,22 @@ void defaultConfig() { | |||
| root->setLevel(log4cxx::Level::getInfo()); | |||
| } | |||
|
|
|||
| // Protects concurrent configuration | |||
| std::mutex configMutex; | |||
There was a problem hiding this comment.
You might consider putting configMutex and initialized in an anonymous namespace.
There was a problem hiding this comment.
It is in anonymous namespace with other code surrounding it (probably too much code in that namespace 🙂 )
This also requires support for pickle in Log class, added simple __reduce__ method.
andy-slac
force-pushed
the
tickets/DM-17874
branch
from
b4f1ff9
to
f890ef8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In addition there are few small improvements:
Logclass now supportspickleso it can work withmultiprocessLog.getChildmethod to make it look more like Pylonlogginglogger, this is also a better replacement for hierarchical "context" management if someone needs that