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

Add ?fail_on_error argument to Lwt_log_core.load_rules, also expose new level_of_string function in the interface #306

Merged
merged 3 commits into from
Dec 23, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 29 additions & 19 deletions src/logger/lwt_log_core.ml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,17 @@ let string_of_level = function
| Error -> "error"
| Fatal -> "fatal"

let level_of_string str =
let str = (String.lowercase [@ocaml.warning "-3"]) str in
Copy link
Collaborator

Choose a reason for hiding this comment

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

You're right to worry about this, but we can't use lowercase_ascii because Lwt still supports 4.02.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, good I kept it intact then.

match str with
| "debug" -> Some Debug
| "info" -> Some Info
| "notice" -> Some Notice
| "warning" -> Some Warning
| "error" -> Some Error
| "fatal" -> Some Fatal
| _ -> None

(* +-----------------------------------------------------------------+
| Patterns and rules |
+-----------------------------------------------------------------+ *)
Expand Down Expand Up @@ -101,31 +112,30 @@ let split pattern =
in
loop 0


let rules = ref []

let load_rules' str =
let load_rules' str fail_on_error =
let rec loop = function
| [] ->
[]
| (pattern, level) :: rest ->
let pattern = split pattern in
match (String.lowercase [@ocaml.warning "-3"]) level with
| "debug" -> (pattern, Debug) :: loop rest
| "info" -> (pattern, Info) :: loop rest
| "notice" -> (pattern, Notice) :: loop rest
| "warning" -> (pattern, Warning) :: loop rest
| "error" -> (pattern, Error) :: loop rest
| "fatal" -> (pattern, Fatal) :: loop rest
| level -> log_intern "invalid log level (%s)" level; loop rest
| [] -> []
| (pattern, level_str) :: rest ->
let pattern = split pattern in
let level = level_of_string level_str in
match level with
| Some level -> (pattern, level) :: loop rest
| None ->
if fail_on_error then raise (Failure "Invalid log rules")
else log_intern "invalid log level (%s)" level_str; loop rest
in
match Lwt_log_rules.rules (Lexing.from_string str) with
| None -> Printf.eprintf "Invalid contents of the LWT_LOG variable\n%!"
| Some l -> rules := loop l
| None ->
if fail_on_error then raise (Failure "Invalid log rules")
else Printf.eprintf "Invalid log rules\n%!"
| Some l -> rules := loop l


let _ =
match try Some(Sys.getenv "LWT_LOG") with Not_found -> None with
| Some str -> load_rules' str
| Some str -> load_rules' str false
| None -> ()

(* +-----------------------------------------------------------------+
Expand Down Expand Up @@ -197,8 +207,8 @@ end

type section = Section.t

let load_rules str =
load_rules' str;
let load_rules ?(fail_on_error=false) str =
load_rules' str fail_on_error;
Section.recompute_levels ()

let add_rule pattern level =
Expand Down
25 changes: 23 additions & 2 deletions src/logger/lwt_log_core.mli
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,30 @@ type section

val string_of_level : level -> string

val load_rules : string -> unit
val level_of_string : string -> level option

val load_rules : ?fail_on_error:bool -> string -> unit
Copy link
Collaborator

@aantron aantron Dec 23, 2016

Choose a reason for hiding this comment

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

Should probably document ?fail_on_error as, besides the exception, there is the non-trivial difference that load_rules ~fail_on_error:false will actually load as many rules as it can, while load_rules ~fail_on_error:true will load nothing if any error is encountered.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree. I'll add a docstring.

(** Reset the rules set when parsing the [LWT_LOG] environment variable using this
string. *)
string.

@param fail_on_error defines if the function will raise Failure if
it encounters a malformed rule
@raise Failure if an invalid rule is found and [fail_on_error] is true

[load_rules] parses the rules string and validates the rules before loading them.
If [fail_on_error] is [true], invalid rules will cause this function to
raise [Failure] and leave existing rules unchanged.
If [fail_on_error] is [false] (this is the default), it tries to load as
many rules as possible and ignore invalid ones.
If the rules string itself cannot be parsed, existing rules are always left
unchanged.

Example:
{[
Lwt_log_core.load_rules ~fail_on_error:true "* -> nosuchlevel" (* Raises Failure *)
Lwt_log_core.load_rules "* -> info"
]}
*)

val add_rule : string -> level -> unit
(** [add_rule pattern level] adds a rule for sections logging
Expand Down