Re: [Tarantool-patches] [PATCH v2 00/10] lua/log: add an ability to setup logger without box.cfg{}

[Oleg Babin <olegrok@tarantool.org>]

Hi! Thanks for your changes.

I have several comments:

1. log.cfg() should be idempotent I think:

```
tarantool> box.cfg{log_level = 2}
---
...

tarantool> box.cfg{log_level = 2}
---
...

tarantool> log.cfg{level = 2}
---
- error: 'builtin/log.lua:207: log: the logger is already initialized'
...
```

2. It would be great to add some validation for input parameters:
```
tarantool> log.level()
---
- error: 'builtin/log.lua:173: bad argument #1 to ''say_set_log_level'' 
(cannot convert
     ''nil'' to ''int'')'
...

tarantool> log.level(-1)
---
...

tarantool> log.level(100000)
---
...

```
Usually functions without arguments return current value of some 
property. See space:format()/:before_replace()/:on_replace functions.
But I believe that we should support log.cfg({level = ...}) instead of 
log.level(...).


Also please don't forget about changelog entry and docbot request.

And seems I've missed this issue 
(https://github.com/tarantool/tarantool/issues/1054) when I asked about 
log.cfg. I think you could add "Closes #1054" as well.

On 26/05/2020 15:46, Cyrill Gorcunov wrote:
> In the series we add an ability to configure logger early
> without calling box.cfg{}. The syntax is the same as
> in box.cfg{} call.
> 
> There was an idea to implement something similar via triggers but
> I think this will require a way more efforts and code redesign,
> so at first lets stick to simplier solution.
> 
> v2:
>   - hide box_api symbols from users
>   - initialize logger via log.cfg() call to look similar
>     to box.cfg
> 
> branch gorcunov/gh-689-logger-2
> issue https://github.com/tarantool/tarantool/issues/689
> 
> Cyrill Gorcunov (10):
>    core/say: drop redundant declarations
>    core/say: drop vsay declaration
>    core/say: do not reconfig early set up logger
>    lua/log: declare say_logger_init and say_logger_initialized
>    lua/log: put string constants to map
>    lua/log: do not allow to set json for boot logger
>    lua/log: declare log as separate variable
>    lua/log: use log module settings inside box.cfg
>    lua/log: allow to configure logging without a box
>    test: use direct log module
> 
>   src/box/lua/load_cfg.lua     |  28 ++++--
>   src/lib/core/say.c           |  18 ++--
>   src/lib/core/say.h           |   8 +-
>   src/lua/log.lua              | 169 ++++++++++++++++++++++++++++++-----
>   test/app-tap/logger.test.lua |  28 +++++-
>   5 files changed, 212 insertions(+), 39 deletions(-)
>