Tarantool development patches archive
 help / color / mirror / Atom feed
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);
>  }

      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