invenia / Memento.jl

A flexible logging library for Julia
https://invenia.github.io/Memento.jl/latest
Other
88 stars 14 forks source link

Allow setting logger propagation in config! #95

Closed iamed2 closed 6 years ago

codecov[bot] commented 6 years ago

Codecov Report

Merging #95 into master will increase coverage by <.01%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #95      +/-   ##
==========================================
+ Coverage   98.93%   98.94%   +<.01%     
==========================================
  Files          11       11              
  Lines         283      284       +1     
==========================================
+ Hits          280      281       +1     
  Misses          3        3
Impacted Files Coverage Δ
src/config.jl 100% <100%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4ec67cb...a24f729. Read the comment docs.

rofinn commented 6 years ago

1) config! is starting to have too many keyword args. 2) I think needing to explicitly create a logger or use setpropagating! is appropriate if you actively want to avoid propagation up the hierarchy. 3) It would have been nice if someone reviewed this before it was merged.

iamed2 commented 6 years ago
  1. Python's logging module considers propagate important enough to have in its config functions

  2. If you want to make a logger that handles custom record types with a custom formatter, it's annoying to have to add the call to setpropagating! to prevent the root logger from erroring on display.

screenshot 2018-08-07 07 15 08
  1. It hasn't been tagged, we can revert it. But I don't think we should.
rofinn commented 6 years ago

Python's logging module considers propagate important enough to have in its config functions

Ahh, okay. Memento.config! was designed after basicConfig which doesn't take a propagate keyword. I'm guessing you were thinking of dictConfig? I guess it doesn't really make sense to distinguish the two in julia as we could just dispatch on an Associative in the future.

iamed2 commented 6 years ago

Ah yes. I think fileConfig also allows this, although I couldn't find a specific example of that.

I think the difference is that basicConfig only operates on the root logger, whereas dictConfig and fileConfig can set properties on any logger.