From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gorcunov@gmail.com>
Received: from mail-lj1-f193.google.com (mail-lj1-f193.google.com
 [209.85.208.193])
 (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits))
 (No client certificate requested)
 by dev.tarantool.org (Postfix) with ESMTPS id 7E5F4469710
 for <tarantool-patches@dev.tarantool.org>;
 Fri, 29 May 2020 13:07:39 +0300 (MSK)
Received: by mail-lj1-f193.google.com with SMTP id z13so1885489ljn.7
 for <tarantool-patches@dev.tarantool.org>;
 Fri, 29 May 2020 03:07:39 -0700 (PDT)
Date: Fri, 29 May 2020 13:07:36 +0300
From: Cyrill Gorcunov <gorcunov@gmail.com>
Message-ID: <20200529100736.GI215590@grain>
References: <20200528100738.221911-1-gorcunov@gmail.com>
 <20200528100738.221911-8-gorcunov@gmail.com>
 <19d91eba-a988-0ee8-6c1b-1b83dca9fce8@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <19d91eba-a988-0ee8-6c1b-1b83dca9fce8@tarantool.org>
Subject: Re: [Tarantool-patches] [PATCH v4 7/8] lua/log: allow to configure
 logging without a box
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Leonid Vasiliev <lvasiliev@tarantool.org>
Cc: tml <tarantool-patches@dev.tarantool.org>

On Fri, May 29, 2020 at 11:41:41AM +0300, Leonid Vasiliev wrote:
> > +local function fmt_list()
> > +    local keyset = {}
> > +    for k in pairs(fmt_str2num) do
> > +        keyset[#keyset + 1] = k
> > +    end
> > +    return table.concat(keyset, ',')
> > +end
> 
> Can this be moved to "lua/log: put string constants to map"?

It will be unused there because only in this patch we start
to print the complete list.

> > +--
> > +-- Load or reload configuration via log.cfg({})
> > +--
> > +-- The syntax is the same as for box.cfg({}).
> > +local function load_cfg(oldcfg, cfg)
> 
> oldcfg - unused argument. I think Sergey will be upset.
> https://github.com/tarantool/tarantool/issues/4681

As far as I understand it is __call semantic, no? IOW,
it is passed when log.cfg{} called as a function.

> >       --
> >       -- Internal API to box module, not for users,
> >       -- names can be changed.
> >       box_api = {
> > -        cfg = log_cfg,
> >           set_log_level = function()
> >               log_level(box.cfg.log_level)
> >           end,
> >           set_log_format = function()
> >               log_format(box.cfg.log_format)
> >           end,
> > +        cfg_apply_default = box_cfg_apply_default,
> > +        cfg_update = box_cfg_update,
> >       },
> 
> Why should we have a special box_module API?
> Maybe we can do all box.cfg dirty stuff in load_cfg.lua.

If you mean to modify log.cfg inside load_cfg.lua directly,
I think it is a bad idea. The variables of a separate module
must never be touched straightforward, this will make code
less maintainable.

> >   }
> > 
> Is it ok and we shouldn't use log.cfg() after box.cfg()?
> 
> └──╼ tarantool
> [70/1585]
> Tarantool 2.5.0-90-g39a946b
> type 'help' for interactive help
> tarantool> log = require("log")
> ---
> ...
> 
> tarantool> box.cfg{log_level = 3}
> 2020-05-29 11:33:09.901 [6926] main/103/interactive C> Tarantool
> 2.5.0-90-g39a946b
> 2020-05-29 11:33:09.901 [6926] main/103/interactive C> log level 3
> 2020-05-29 11:33:09.974 [6926] main/103/interactive C> leaving orphan mode
...

Thanks, will take a look.