From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 8308F6EC5F; Tue, 13 Apr 2021 20:57:16 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 8308F6EC5F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1618336636; bh=3jBfzztWWhacsQqozwXOUQaCfZv+bMY2v+XmpsX8fx4=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=nhKuMpNB3ZH18vlueGC/ipocjhT7x+x+OM66UEQQ3/rV6bbFWDE65rGEGgH9EEQr2 Wl6XnZCGBTcVJr7bX0iqDiSmJPQqZ4FsE8SUqupoGV9IpPCjmXEk1Kd+lvRQO9Px3y RpcyuPPRaxRDf5BJ945nBP5PQKq4lRM7whCbD1EM= Received: from mail-lj1-f170.google.com (mail-lj1-f170.google.com [209.85.208.170]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 1E7A76EC5F for ; Tue, 13 Apr 2021 20:57:15 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 1E7A76EC5F Received: by mail-lj1-f170.google.com with SMTP id p23so16691285ljn.0 for ; Tue, 13 Apr 2021 10:57:15 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=6AZE/Nwoa3RQmEp/Z2FCuYSwkejR3fgCrupaGoudmN0=; b=VzoXW3qdLjXyzsOIsuo7WgSen4HrL+4WAoQRBawjU/J+//WC+ZsL8Ttf+WLgqNqjfV 8dWT2mYpJoNctwTPEiy3CLGEMLkucRFaVzDEjCjUMmCwSAyFSpxKiULAzdxXibuY0dCO DF5s8SxvKf7hRss1XTf3DQNbPqHnNv3r7JOLMAm4XxwSdtRWhfuJtMINPvq0MGZ6+usY zO42xacAvvYgYftNi7F506wqWF5jeyp+8aiXdRbvYmEi8+HfTl2RgHz5yjTtIOrGfWf3 6hFrznb7gOZMdvqbUHp9keLhRpg7owgKly5EqTDJ6YeTtAdd4Rl1An2EV3Uexi5lo81P 2BDg== X-Gm-Message-State: AOAM530QXmHatjrawRtOBYswSVm8yEeCQ4xcH40xQKu9YwJ7GZrTQeP3 gRrM7PiSLe9dfDTbYr4OUpAjF1HC3bE= X-Google-Smtp-Source: ABdhPJxk00nK619RWtHxj2P+wvvKqS/55Dn1kYm5yE/timkXe4tN/FkanSszLylYA7VRB6h9crl7nA== X-Received: by 2002:a05:651c:2001:: with SMTP id s1mr6020528ljo.236.1618336633847; Tue, 13 Apr 2021 10:57:13 -0700 (PDT) Received: from grain.localdomain ([5.18.199.94]) by smtp.gmail.com with ESMTPSA id o29sm98808lfn.95.2021.04.13.10.57.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 13 Apr 2021 10:57:12 -0700 (PDT) Received: by grain.localdomain (Postfix, from userid 1000) id AB29B560162; Tue, 13 Apr 2021 20:57:11 +0300 (MSK) Date: Tue, 13 Apr 2021 20:57:11 +0300 To: Leonid Vasiliev , Roman Khabibov Cc: TML Message-ID: References: <20210413124559.47410-1-roman.habibov@tarantool.org> <20210413124559.47410-3-roman.habibov@tarantool.org> <28e3e145-d86b-bf21-8a40-73da577bfbff@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <28e3e145-d86b-bf21-8a40-73da577bfbff@tarantool.org> User-Agent: Mutt/2.0.5 (2021-01-21) Subject: Re: [Tarantool-patches] [PATCH v2 2/2] box: set cfg via environment variables X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Cyrill Gorcunov via Tarantool-patches Reply-To: Cyrill Gorcunov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" 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 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 --- 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