Skip to content

MultifilesLogger.jl#1

Open
tencnivel wants to merge 11 commits into
Seelengrab:masterfrom
tencnivel:master
Open

MultifilesLogger.jl#1
tencnivel wants to merge 11 commits into
Seelengrab:masterfrom
tencnivel:master

Conversation

@tencnivel
Copy link
Copy Markdown

@tencnivel tencnivel commented Jul 14, 2019

MultifilesLogger allows to have several log files.
Every log file is associated to a vector of Tuple{Module, LogLevel} using the struct FileDefForMultifilesLogger.

If a module is not associated to an IO, getIOLevelTuple!() will try to associate the module with the IO of a parent module.

NOTE: test/runtests-MultifilesLogger.jl would require some properly written tests. My first attempt to mimic the test sets of FileLogger has failed (the log files are empty, I don't understand why).

@tencnivel tencnivel marked this pull request as ready for review July 14, 2019 17:19
Comment thread test/runtests-MultifilesLogger.jl Outdated
Comment thread test/runtests-MultifilesLogger.jl Outdated
Remove some comments
Copy link
Copy Markdown
Owner

@Seelengrab Seelengrab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of distinguishing between modules when logging, but I think it would be a good idea to incorporate the ideas of logging ranges from the FileLogger and generic IOLogger as well into this. That way different modules could be logged with different priorities and you can easily specify different ranges of log levels for different modules, with no logging done being the default.

Comment thread src/MultifilesLogger.jl Outdated
Dict{Module, Tuple{IO, LogLevel}}(),
Dict{Any, Int}(),
flush);
createIOs!(x);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe do this lazily in handle_message on a per module basis in order to prevent opening too many files at creation of the logger even though they may not be needed at all.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it's not consistent with the rest of the package but I find it more user friendly to have the files initialized at the instantiation of the logger. Like this we can check that everything is fine (eg.: check that files are created, check that files are emptied for IO with 'append' set to false).

Comment thread src/MultifilesLogger.jl Outdated
Comment thread src/FileLogger.jl Outdated
Comment thread src/MultifilesLogger.jl Outdated
- Move the 'flush' property from 'MultifilesLogger' to FileDefForMultifilesLogger
- Do not throw an error if we cannot find an IO for a given module
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants