From: Vladimir Davydov <vdavydov.dev@gmail.com> To: Olga Arkhangelskaia <krishtal.olja@gmail.com> Cc: tarantool-patches@freelists.org, Olga Arkhangelskaia <arkholga@tarantool.org> Subject: Re: [tarantool-patches] [PATCH v3] Fixed non-informative error messages for log conf. Date: Wed, 8 Aug 2018 16:01:45 +0300 [thread overview] Message-ID: <20180808130000.oova73yhnk7fttw7@esperanza> (raw) In-Reply-To: <20180807201114.43870-1-krishtal.olja@gmail.com> On Tue, Aug 07, 2018 at 11:11:14PM +0300, Olga Arkhangelskaia wrote: > diff --git a/src/box/box.cc b/src/box/box.cc > index 62fd05468..9601697f6 100644 > --- a/src/box/box.cc > +++ b/src/box/box.cc > @@ -354,15 +354,12 @@ apply_initial_join_row(struct xstream *stream, struct xrow_header *row) > static void > box_check_say() > { > + enum say_logger_type type = SAY_LOGGER_STDERR; /* default */ > const char *log = cfg_gets("log"); > - if (log == NULL) > - return; > - enum say_logger_type type; > - if (say_parse_logger_type(&log, &type) < 0) { > + if (log != NULL && say_parse_logger_type(&log, &type) < 0) { > tnt_raise(ClientError, ER_CFG, "log", > diag_last_error(diag_get())->errmsg); > } > - > if (type == SAY_LOGGER_SYSLOG) { > struct say_syslog_opts opts; > if (say_parse_syslog_opts(log, &opts) < 0) { > @@ -379,25 +376,24 @@ box_check_say() > const char *log_format = cfg_gets("log_format"); > enum say_format format = say_format_by_name(log_format); > if (format == say_format_MAX) > - diag_set(ClientError, ER_CFG, "log_format", > + tnt_raise(ClientError, ER_CFG, "log_format", > "expected 'plain' or 'json'"); > if (type == SAY_LOGGER_SYSLOG && format == SF_JSON) { > - tnt_raise(ClientError, ER_ILLEGAL_PARAMS, "log, log_format"); > + tnt_raise(ClientError, ER_CFG, "log_format", > + "'json' can't be used with syslog logger"); > } > int log_nonblock = cfg_getb("log_nonblock"); > - if (log_nonblock == 1 && type == SAY_LOGGER_FILE) { > - tnt_raise(ClientError, ER_ILLEGAL_PARAMS, "log, log_nonblock"); > + if (log_nonblock == 1 && > + (type == SAY_LOGGER_FILE || type == SAY_LOGGER_STDERR)) { > + tnt_raise(ClientError, ER_CFG, "log_nonblock", > + "the option is incompatible with file/stderr logger"); > } > } > > static enum say_format > box_check_log_format(const char *log_format) > { > - enum say_format format = say_format_by_name(log_format); > - if (format == say_format_MAX) > - tnt_raise(ClientError, ER_CFG, "log_format", > - "expected 'plain' or 'json'"); > - return format; > + return say_format_by_name(log_format); > } Apparently, you don't need box_check_log_format() anymore as it simply calls say_format_by_name() with the same argument. Please remove it and call say_format_by_name() in box_set_log_format() directly. Other than that, the patch looks good to me. > > static void > @@ -745,6 +741,7 @@ box_set_log_level(void) > void > box_set_log_format(void) > { > + box_check_say(); > enum say_format format = box_check_log_format(cfg_gets("log_format")); > say_set_log_format(format); > }
prev parent reply other threads:[~2018-08-08 13:01 UTC|newest] Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-08-07 20:11 Olga Arkhangelskaia 2018-08-08 13:01 ` Vladimir Davydov [this message]
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20180808130000.oova73yhnk7fttw7@esperanza \ --to=vdavydov.dev@gmail.com \ --cc=arkholga@tarantool.org \ --cc=krishtal.olja@gmail.com \ --cc=tarantool-patches@freelists.org \ --subject='Re: [tarantool-patches] [PATCH v3] Fixed non-informative error messages for log conf.' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox