Add option to make config value more stricitive

[ganmacs]

Is your feature request related to a problem? Please describe.

ref: #2607

Describe the solution you'd like

#2615 (comment)

We should improve this point to use Integer() or others.
This change has compatibility probolem so it will be implemented in v2 or via configuration.

Additional context

[repeatedly]

In addition, support nil is better.
I sometimes see users use "#{ENV['SOME_ENV_VAR']}" in the configuration and they expect nil is passed when no envvar. Currently, this is converted into "".

[ashie]

In #2685, I've implemented integer, float and bool cases.

In addition, support nil is better.
I sometimes see users use "#{ENV['SOME_ENV_VAR']}" in the configuration and they expect nil is passed when no envvar. Currently, this is converted into "".

I'm now considering this case.
IMHO it shouldn't be treated as nil, empty string "" is the correct behavior.
I think their expectation is wrong because the document says

You can evaluate the Ruby code with #{} in " quoted string.

and #{ENV['SOME_ENV_VAR']} is evaluated as empty string in actual Ruby code.

irb(main):001:0> "#{ENV['SOME_ENV_VAR']}"
=> ""

so I don't expect nil for it.
If the literal is ENV['SOME_ENV_VAR'] (without #{}), I'll expect nil for it though:

irb(main):001:0> ENV['SOME_ENV_VAR']
=> nil

[ashie]

Is there any other case that nil should be supported?

[ganmacs]

In addition, support nil is better.

@repeatedly

What does this expect in this feature?
nil can pass the validation even if the value is expected to be something type(String, Integer or whatever)?

[cosmo0920]

Is there any other case that nil should be supported?

Handling for ENV['NONEXISTENT'] #=> nil case?

[repeatedly]

I think their expectation is wrong because the document says

Yes but many users think it becomes nil and this is why users use username "#{ENV['XXX_USER'] || nil}" in many places. Of course, this doesn't work as expected.

Other apporch is adding <if ruby cond> approch to support ENV usecase like below.

<if ENV['XXX_USER']>
  username "#{ENV['XXX_USER']}"
</if>

[ashie]

Thanks for your comment.

Other apporch is adding approch to support ENV usecase like below.

<if ENV['XXX_USER']>
  username "#{ENV['XXX_USER']}"
</if>

You mean that the parameter should be treated as "unspecified" (use the default value) if the value is 'nil', don't you? It's an interesting feature.

I think it should be treated as a different issue, it's a different feature from #2685.
We should determine the syntax for nil.
I'll file a new issue after completing #2685.

[ganmacs]

I think it should be treated as a different issue, it's a different feature from #2685.

+1

@repeatedly Could you tell me your thought? I don't still understand what "support nil" is.
#2616 (comment)

[ashie]

BTW

this is why users use username "#{ENV['XXX_USER'] || nil}" in many places. Of course, this doesn't work as expected.

Following dirty hacks may be effective for such case...

   username "#{ENV['XXX_USER'] || raise("no username")}"

   # launcher can detect it by the exit status
   username "#{ENV['XXX_USER'] || exit(123)}"

[ganmacs]

Ruby has Hash#fetch(https://apidock.com/ruby/Hash/fetch)
Isn't username "#{ENV.fetch('XX_USER')}" enough for such case?

[repeatedly]

Above works? I tested it and fluentd stopped by /path/to/lib/fluent/config/literal_parser.rb:171:in `instance_eval': key not found: "NUM" (KeyError) exception.

You mean that the parameter should be treated as "unspecified" (use the default value) if the value is 'nil', don't you? It's an interesting feature.

No. I show nil because almost plugins use nil for default value like below.

config_param :str_param, :string, :default => nil
config_param :num_param, :integer, :default => nil

Currently, str_param "#{ENV['STR'] || nil}" becomes '' and num_param "#{ENV['NUM'] || nil}" becomes 0 in plugin code but users think this becomes nil. We need to fix this gap.

I just notice helper method is also one adhoc approach like below:

num_param "#{ENV['NUM'] || use_default}"  # raise specific exception to skip value processing. use default in config_param.
or
num_param "#{ENV['NUM'] || use_nil}"

[ashie]

num_param "#{ENV['NUM'] || use_default}"  # raise specific exception to skip value processing. use default in config_param.
or
num_param "#{ENV['NUM'] || use_nil}"

I'm now confirming this concept in another branch:
ashie@f7b5914

Implementing 'use_nil' seems possible, but 'use_default' seems difficult because default value isn't determined while parsing the config file. It's determined at Configurable::configure which is run after parsing config file (receives a parsed config as an argument).

[ashie]

but 'use_default' seems difficult because default value isn't determined while parsing the config file. It's determined at Configurable::configure which is run after parsing config file (receives a parsed config as an argument).

Now I've noticed that it's possible by returning ":default" or something from LiteralParser and processing it at Config::STRING_TYPE or other equivalents.

[ashie]

I'm now confirming this concept in another branch:
ashie@f7b5914

I've merged these changes to #2685, since they are based on changes in #2685.