From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <vdavydov.dev@gmail.com>
Date: Wed, 24 Jul 2019 12:01:22 +0300
From: Vladimir Davydov <vdavydov.dev@gmail.com>
Subject: Re: [tarantool-patches] [PATCH] Initial box.cfg call logs changes now
Message-ID: <20190724090122.GA11394@esperanza>
References: <CA+JJC5sCc2d6QncKnMEjgfFs_WEtcCJCchZ=NM8DuPxZbwWdXw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CA+JJC5sCc2d6QncKnMEjgfFs_WEtcCJCchZ=NM8DuPxZbwWdXw@mail.gmail.com>
To: Maria K <marianneliash@gmail.com>
Cc: tarantool-patches@freelists.org, georgy@tarantool.org
List-ID: <tarantool-patches.dev.tarantool.org>

The patch doesn't apply:

Applying: Initial box.cfg call logs changes now
error: corrupt patch at line 25
error: could not build fake ancestor
Patch failed at 0001 Initial box.cfg call logs changes now

Please use git-send-email (or any other MUA that doesn't wrap lines) and
set your full name.

You'll find the comments regarding the patch itself inline.

On Mon, Jul 22, 2019 at 12:18:36PM +0300, Maria K wrote:
> In contrast to subsequent calls, the initial call to box.cfg didn't log
> configuration changes to the default state. As a result, by looking at
> a log file we couldn't tell which configuration was being used.
> 
> Closes #4236
> ---
>  src/box/lua/load_cfg.lua   |   6 +-
>  test/box/cfg.result        | 900 +++++++++++++++++++------------------
>  test/box/cfg.test.lua      |   9 +
>  test/box/lua/cfg_test5.lua |  10 +
>  4 files changed, 497 insertions(+), 428 deletions(-)
>  create mode 100644 test/box/lua/cfg_test5.lua
> 
> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
> index 83e99e854..d402468ab 100644
> --- a/src/box/lua/load_cfg.lua
> +++ b/src/box/lua/load_cfg.lua
> @@ -492,8 +492,10 @@ local function load_cfg(cfg)
>      private.cfg_load()
>      for key, fun in pairs(dynamic_cfg) do
>          local val = cfg[key]
> -        if val ~= nil and not dynamic_cfg_skip_at_load[key] then
> -            fun()
> +        if val ~= nil then
> +            if not dynamic_cfg_skip_at_load[key] then
> +                fun()
> +            end

This wouldn't work for all configuration options. For instance,

  box.cfg{row_per_wal = 1000}

doesn't log anything.

>              if not compare_cfg(val, default_cfg[key]) then
>                  log.info("set '%s' configuration option to %s", key,
> json.encode(val))
>              end
> diff --git a/test/box/cfg.result b/test/box/cfg.result
> index d85ec634c..cdca64ef0 100644
> --- a/test/box/cfg.result
> +++ b/test/box/cfg.result
> @@ -1,538 +1,586 @@
> +-- test-run result file version 2
>  env = require('test_run')
> ----
> -...
> + | ---
> + | ...
>  test_run = env.new()
> ----
> -...
> + | ---
> + | ...
>  test_run:cmd("push filter '(error: .*)\\.lua:[0-9]+: ' to '\\1.lua:<line>:
> '")
> ----
> -- true
> -...
> + | ---
> + | - true
> + | ...
>  box.cfg.nosuchoption = 1

It looks like you deleted the result file, which made test-run
generate a new result using the new format. Please don't do that,
because this makes the patch impossible to review. Instead update
the result file with the reject file written by test-run.

> diff --git a/test/box/cfg.test.lua b/test/box/cfg.test.lua
> index eddeab126..56ccb6767 100644
> --- a/test/box/cfg.test.lua
> +++ b/test/box/cfg.test.lua
> @@ -132,3 +132,12 @@ box.cfg{net_msg_max = old + 1000}
>  box.cfg{net_msg_max = old}
> 
>  test_run:cmd("clear filter")
> +
> +--
> +-- gh-4236: initial box.cfg{} call did not log changes to default state
> +--
> +test_run:cmd('create server cfg_tester6 with script =
> "box/lua/cfg_test5.lua"')
> +test_run:cmd("start server cfg_tester6")
> +test_run:grep_log('cfg_tester6', 'set \'vinyl_memory\' configuration
> option to 1073741824', 1000)
> +test_run:cmd("stop server cfg_tester6")
> +test_run:cmd("cleanup server cfg_tester6")
> diff --git a/test/box/lua/cfg_test5.lua b/test/box/lua/cfg_test5.lua
> new file mode 100644
> index 000000000..e3eb87392
> --- /dev/null
> +++ b/test/box/lua/cfg_test5.lua
> @@ -0,0 +1,10 @@
> +#!/usr/bin/env tarantool
> +os = require('os')
> +
> +box.cfg{
> +    listen              = os.getenv("LISTEN"),
> +    vinyl_memory = 1024 * 1024 * 1024
> +}
> +
> +require('console').listen(os.getenv('ADMIN'))
> +box.schema.user.grant('guest', 'read,write,execute', 'universe')
> \ No newline at end of file

Please configure your editor so that it adds a new line at the end of a
file.