[Tarantool-patches] [PATCH 6/9] cfg: introduce 'deprecated option' feature

[Oleg Babin]

Thanks for your fixes. LGTM!

On 11/02/2021 01:34, Vladislav Shpilevoy wrote:
> Thanks for the review!
>
> On 10.02.2021 09:59, Oleg Babin wrote:
>> Thanks for your patch!
>>
>> Is it possible to extend log message to "Option is deprecated and has no effect anymore"?
> Good idea. See the diff in this commit.
>
> ====================
> diff --git a/vshard/cfg.lua b/vshard/cfg.lua
> index 28c3400..f7d5dbc 100644
> --- a/vshard/cfg.lua
> +++ b/vshard/cfg.lua
> @@ -61,7 +61,13 @@ local function validate_config(config, template, check_arg)
>           local expected_type = template_value.type
>           if template_value.is_deprecated then
>               if value ~= nil then
> -                log.warn('Option "%s" is deprecated', name)
> +                local reason = template_value.reason
> +                if reason then
> +                    reason = '. '..reason
> +                else
> +                    reason = ''
> +                end
> +                log.warn('Option "%s" is deprecated'..reason, name)
>               end
>           elseif value == nil then
>               if not template_value.is_optional then
> ====================
>
> And in the next commit:
>
> ====================
> diff --git a/vshard/cfg.lua b/vshard/cfg.lua
> index f7dd4c1..63d5414 100644
> --- a/vshard/cfg.lua
> +++ b/vshard/cfg.lua
> @@ -252,6 +252,7 @@ local cfg_template = {
>       },
>       collect_bucket_garbage_interval = {
>           name = 'Garbage bucket collect interval', is_deprecated = true,
> +        reason = 'Has no effect anymore'
>       },
>       collect_lua_garbage = {
>           type = 'boolean', name = 'Garbage Lua collect necessity',
>
> ====================
>
>> Also for some options could be useful: "Option is deprecated, use ... instead" (e.g. for "weights").
> With the updated version I can specify any 'reason'. Such as
> 'has no affect', 'use ... instead', etc.
>
>> Seems it should be more configurable and gives some hint for user to do.
>>
>>
>> On 10/02/2021 02:46, Vladislav Shpilevoy wrote:
>>> Some options in vshard are going to be eventually deprecated. For
>>> instance, 'weigts' will be renamed, 'collect_lua_garbage' may be
>> typo: weigts -> weights
> Fixed. See the full new patch below.
>
> ====================
>      cfg: introduce 'deprecated option' feature
>      
>      Some options in vshard are going to be eventually deprecated. For
>      instance, 'weights' will be renamed, 'collect_lua_garbage' may be
>      deleted since it appears not to be so useful, 'sync_timeout' is
>      totally unnecessary since any 'sync' can take a timeout per-call.
>      
>      But the patch is motivated by 'collect_bucket_garbage_interval'
>      which is going to become unused in the new GC algorithm.
>      
>      New GC will be reactive instead of proactive. Instead of periodic
>      polling of _bucket space it will react on needed events
>      immediately. This will make the 'collect interval' unused.
>      
>      The option will be deprecated and eventually in some far future
>      release its usage will lead to an error.
>      
>      Needed for #147
>
> diff --git a/vshard/cfg.lua b/vshard/cfg.lua
> index 1ef1899..f7d5dbc 100644
> --- a/vshard/cfg.lua
> +++ b/vshard/cfg.lua
> @@ -59,7 +59,17 @@ local function validate_config(config, template, check_arg)
>           local value = config[key]
>           local name = template_value.name
>           local expected_type = template_value.type
> -        if value == nil then
> +        if template_value.is_deprecated then
> +            if value ~= nil then
> +                local reason = template_value.reason
> +                if reason then
> +                    reason = '. '..reason
> +                else
> +                    reason = ''
> +                end
> +                log.warn('Option "%s" is deprecated'..reason, name)
> +            end
> +        elseif value == nil then
>               if not template_value.is_optional then
>                   error(string.format('%s must be specified', name))
>               else
>
> ====================