Skip to content
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

appenders can't be removed using (merge-config! {:appenders {:appender-name nil}}) #163

Closed
andreasthoelke opened this issue Apr 15, 2016 · 4 comments

Comments

@andreasthoelke
Copy link

andreasthoelke commented Apr 15, 2016

Using [com.taoensso/timbre "4.3.1"] in a fresh project, when I run

(-> (timbre/merge-config! {:appenders {:println nil}})
    :appenders
    :println)

I get:

{:enabled? true,
 :async? false,
 :min-level nil,
 :rate-limit nil,
 :output-fn :inherit,
 :fn #object[taoensso.timbre.appenders.core$println_appender$fn__24977 0x7e0d7a88 "taoensso.timbre.appenders.core$println_appender$fn__24977@7e0d7a88"]}

instead of nil, as I would expect.

Similarly appenders that I add with merge-config!

(timbre/merge-config! {:appenders {:spit (appenders/spit-appender {:fname "log.txt"})}})

can't be removed via merge-config!

(-> (timbre/merge-config! {:appenders {:spit nil}})
    :appenders
    :spit)

returns:

{:enabled? true,
 :async? false,
 :min-level nil,
 :rate-limit nil,
 :output-fn :inherit,
 :fn #object[taoensso.timbre.appenders.core$println_appender$fn__24977 0x7e0d7a88 "taoensso.timbre.appenders.core$println_appender$fn__24977@7e0d7a88"]}

With regards to disabling (vs. removing) an appender (which seemed to be the problem here [https://github.com//issues/161]), you can still do

(timbre/merge-config! {:appenders {:println {:enabled? false}}})

I.e.

(-> (timbre/merge-config! {:appenders {:println {:enabled? false}}})
    :appenders
    :println)

returns enabled? false:

{:enabled? false,
 :async? false,
 :min-level nil,
 :rate-limit nil,
 :output-fn :inherit,
 :fn #object[taoensso.timbre.appenders.core$println_appender$fn__24977 0x7e0d7a88 "taoensso.timbre.appenders.core$println_appender$fn__24977@7e0d7a88"]}

as expected.

@ptaoussanis
Copy link
Member

ptaoussanis commented Apr 16, 2016

Hey Andreas, thanks so much for the detailed report! Have pushed a fix that'll go out with the next Timbre release.

In the meantime, your suggestion to use (timbre/merge-config! {:appenders {:println {:enabled? false}}}) is good. Otherwise folks can just mod their Timbre config map manually using the standard Clojure API, so something like: (timbre/swap-config! assoc-in [:appenders :println] nil), etc.

Whatever's most convenient, there's no magic happening underneath; Timbre's config is just a standard map (at timbre/*config*). To get rid of an appender you just need to remove the appender key (or set it's :enabled? val to false).

Cheers :-)

@andreasthoelke
Copy link
Author

andreasthoelke commented Apr 16, 2016

Thank you Peter! Yep, the config approach seems simple and super flexible.

Good point in using

(timbre/swap-config! assoc-in [:appenders :println :enabled?] false)

over

(timbre/merge-config! {:appenders {:println {:enabled? false}}})

as the former makes the path to the config param more readable.
And of cause you can have a helper like

(def set-log-config-param! (partial timbre/swap-config! assoc-in))

and then be even more consise and also compatible with the timbre v3.4.0 set-config! syntax:

(set-log-config-param! [:appenders :println :enabled?] false)

I wonder if a similar timbre/set-config-param! would make sense to ease transitioning
log configs between v3 and v4?

@ptaoussanis
Copy link
Member

Hi Andreas, no problem :-)

I wonder if a similar timbre/set-config-param! would make sense to ease transitioning
log configs between v3 and v4?

Do you think this is a common problem? Is it because new users don't know how to use assoc-in? (Sincere question).

@andreasthoelke
Copy link
Author

andreasthoelke commented Apr 17, 2016

Yeah, I think (timbre/swap-config! assoc-in [:appenders :println :enabled?] false) will do fine. It makes it clear what is happening under the hood.
Thanks again for the considered response!

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

No branches or pull requests

2 participants