Re: [Tarantool-patches] [PATCH v2 2/2] box: set cfg via environment variables

[Cyrill Gorcunov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Guys, I must confess I don't like dropping 'module' from template_cfg,
the reason it has been introduced in first place is that modules might
be used _before_ the box.cfg{} even called. So that the modules should
carry own config and provide it back to the load_lua code.

Currently we have one such 'early' module -- it is logger. But we might
extend it in future. Moreover spreading types here and there won't make
code anyhow easier to read and modify.

Also the question raises -- the environment variables are considered
only at box.cfg{} call, but should not be they also handled by modules
which support early use? If yes -- then we need to add such support
into the logger code. If no -- then it should be pointed in documentation
that log module ignores env variables if set up before box.cfg{} call.
Anoter option -- defer such support to the next release.

Now back to types. Current Roman's code test for basic types from @template_cfg
which means later when these values are setting into real @cfg variable they
are passing same testing again

prepare_cfg
  ...
            local good_types = string.gsub(template_cfg[k], ' ', '');
            if (string.find(',' .. good_types .. ',', ',' .. type(v) .. ',') == nil) then
                box.error(box.error.CFG, readable_name, "should be one of types "..
                    template_cfg[k])
            end

so the question is why do we need to do this double work? As far as I understand
the only thing what we need to do is to *fetch* data from environment variables
and set them into @cfg variable. Then loader will process every entry and report
an error if something goes wrong. Though I think we might need to dequote variables
just like it is done in the patch.

Now back to types. If you really think that we still need the validation of types
on the stage when we fetch the values from env variables then please don't revert
the commit with 'module' keyword but rather provide the types separately so
we rework them if needed. Modules should be extendable not squashed into defaults.

Say for now we could use
---
From: Cyrill Gorcunov <gorcunov@gmail.com>
Date: Tue, 13 Apr 2021 20:50:14 +0300
Subject: [PATCH] cfg: provide types for logger options

This needed for fast type check when fetching
options from environment variable.

Part-of #5602

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/box/lua/load_cfg.lua | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index d31c9eb2c..7f57fcbb1 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -118,6 +118,18 @@ local module_cfg = {
     log_format          = log.box_api,
 }
 
+-- cfg types for modules, probably better to
+-- provide some API with type enumeration or
+-- similar. Currently it has use for environment
+-- processing only.
+local module_cfg_type = {
+    -- logging
+    log                 = 'string',
+    log_nonblock        = 'boolean',
+    log_level           = 'number, string',
+    log_format          = 'string',
+}
+
 -- types of available options
 -- could be comma separated lua types or 'any' if any type is allowed
 local template_cfg = {
@@ -686,6 +698,10 @@ end
 local function process_option_value(option, raw_value)
     local param_type = template_cfg[option]
 
+    if param_type == 'module' then
+        param_type = module_cfg_type[option]
+    end
+
     -- May be peculiar to "feedback*" parameters.
     if param_type == nil then
         return nil
-- 
2.30.2