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

Allow log levels to be configured per-source. #159

Closed
wants to merge 2 commits into from

Conversation

jsthomas
Copy link
Contributor

This change proposes an update to Dream.initialize_log that allows log levels to be specified for individual log sources.

This is motivated by comments in #90. There, @aantron mentioned the need for a way to configure log level per source, which would be useful for working with logs related to flash messages.

Example Usage

let () =
  Dream.initialize_log
    ~level:`Warning
    ~custom_levels:[("dream.flash", `Info)]   (* NEW *)
    ();
  Dream.run
  ...

The change introduces an optional argument called custom_levels that accepts an association list. Sources not named in ~custom_levels are set to ~level, so Dream.initialize_log retains its original behavior when ~custom_levels is not used.

@aantron
Copy link
Owner

aantron commented Oct 17, 2021

I would suggest a separate API like

Dream.set_log_level : string -> [< log_level ] -> unit

That would allow different modules to "locally" set log levels for sources that are relevant to them, and not have these concerns centralized in the log initializer function (which might not even be called for any other purpose).

It might also be good to add ?level to Dream.sub_log, so:

Dream.sub_log : ?level:[< log_level ] -> string -> sub_log

@jsthomas
Copy link
Contributor Author

jsthomas commented Nov 3, 2021

In the interest of simplicity, I'm going to close this PR and open a new one that has the latest changes on the master branch.

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.

None yet

2 participants