[Tarantool-patches] [PATCH v2 00/10] lua/log: add an ability to setup logger without box.cfg{}
Oleg Babin
olegrok at tarantool.org
Tue May 26 16:42:52 MSK 2020
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(-)
>
More information about the Tarantool-patches
mailing list