[Freeipa-devel] [PATCH 0051] IPA server and replica installers can accept options from config file

Wed Aug 26 10:00:50 UTC 2015

On 26.8.2015 11:48, Jan Cholasta wrote:
> On 26.8.2015 10:51, Petr Spacek wrote:
>> On 30.7.2015 08:55, Jan Cholasta wrote:
>>> Dne 29.7.2015 v 17:43 Petr Vobornik napsal(a):
>>>> On 07/29/2015 05:13 PM, Martin Babinsky wrote:
>>>>> On 07/29/2015 01:25 PM, Jan Cholasta wrote:
>>>>>> Dne 29.7.2015 v 12:20 Martin Babinsky napsal(a):
>>>>>>> Initial attempt to implement
>>>>>>> https://fedorahosted.org/freeipa/ticket/4517
>>>>>>>
>>>>>>> Some points to discuss:
>>>>>>>
>>>>>>> 1.) name of the config entries: currently the option names are derived
>>>>>>> from CLI options but have underscores in them instead of dashes. Maybe
>>>>>>> keeping the CLI option names also for config entries will make it
>>>>>>> easier
>>>>>>> for the user to transfer their CLI options from scripts to config
>>>>>>> files.
>>>>>>
>>>>>> NACK. There is no point in generating config names from CLI names, which
>>>>>> are generated from knob names - use knob names directly.
>>>>>>
>>>>> The problem is that in some cases the  cli_name does not map directly to
>>>>> knob name, leading in different naming of CLI options and config
>>>>> entries, confusion and mayhem.
>>>
>>> What works for CLI may not work for config files and vice versa. For example,
>>> this works for CLI:
>>>
>>>      --no-ntp
>>>      --no-forwarders
>>>      --forwarder 1.2.3.4 --forwarder 5.6.7.8
>>>
>>> but this works better in config file:
>>>
>>>      ntp = False
>>>      forwarders =
>>>      forwarders = 1.2.3.4, 5.6.7.8
>>
>> Personally I would say that Honza's approach is more eye-candy but IMHO *not*
>> more usable - and I prefer usability. Alexander's approach requires no other
>> documentation that `ipa-server-install --help` or even better just
>> copy&pasting arguments users already have in scripts to a file.
>>
>> In this case I believe that using anything incompatible with CLI arguments is
>> not worth because it requires yet another layer of documentation to make it
>> usable.
>>
>> BTW GnuPG does the very same thing as Alexander mentioned with
>> .gnupg/gpg.conf, i.e. the config file simply accepts all options from command
>> line, with the same names and parameters - and that that is it.
> 
> Sorry, but no. The installers are supposed to be callable from many different
> kinds of often incompatible environments. How exactly would you imagine e.g. a
> Python API to look like given it should use the same conventions as CLI? Like
> this:
> 
>     server_install(['no_ntp', ('forwarder', '1.2.3.4'), ('forwarder',
> '5.6.7.8')])
> 
> ? I would very much prefer if it looked like this:
> 
>     server_install(ntp=False, forwarders=['1.2.3.4', '5.6.7.8'])
> 
> which is pretty much the same I suggested for config files and better reflects
> the semantics of setting configuration options.

I'm just saying that:
1. API & user-interface on CLI are not the same, so there is no need to
strictly use the same names in API and CLI (which we apparently do not do,
compare --help and internal knobs).

2. User interface self-consistency (CLI options vs. configuration file) is
more important that consistency between config file and API.

Petr^2 Spacek

>>>>> These are some offenders from `ipaserver/install/server.py`:
>>>>> http://fpaste.org/249424/18226114/
>>>>>
>>>>> On the other hand, this can be an incentive to finally put an end to
>>>>> inconsistent option/knob naming across server/replica/etc. installers.
>>>
>>> Yes please.
>>>
>>>>
>>>> If the names are different than cli names, then they should be made
>>>> discoverable somehow or be documented.
>>>
>>> IMHO documenting them is easy.
>>>
>>>>
>>>>>>>
>>>>>>> 2.) Config sections: there is currently only one valid section named
>>>>>>> '[global]' in accordance with the format of 'default.conf'. Should we
>>>>>>> have separate sections equivalent to option groups in CLI (e.g.
>>>>>>> [basic],
>>>>>>> [certificate system], [dns])?
>>>>>>
>>>>>> No, because they would have to be maintained forever. For example, some
>>>>>> options are in wrong sections and we wouldn't be able to move them.
>>>>>>
>>>>> I'm also more inclined to a single section, at least for now since we
>>>>> are pressed for time with this RFE.
>>>>>
>>>>> That's not to say that we should ditch Alexander's idea about separate
>>>>> sections with overrides for different hosts. We should consider it as a
>>>>> future enhancement to this feature once the basic plumbing is in place.
>>>
>>> Right.
>>>
>>>>>>>
>>>>>>> 3.) Handling of unattended mode when specifying a config file:
>>>>>>> Currently there is no connection between --config-file and unattended
>>>>>>> mode. So when you run ipa-server-install using config file, you still
>>>>>>> get asked for missing stuff. Should '--config-file' automatically imply
>>>>>>> '--unattended'?
>>>>>>
>>>>>> The behavior should be the same as if you specified the options on the
>>>>>> command line. So no, --config-file should not imply --unattended.
>>>>>>
>>>>> That sound reasonable. the code behaves this way already so no changes
>>>>> here.
>>>>>
>>>>>>>
>>>>>>> There are probably other issues to discuss. Feel free to write
>>>>>>> email/ping me on IRC.
>>>>>>>
>>>>>>
>>>>>> (I haven't looked at the patch yet.)
>>>>>>
>>>>> Please take a look at it ASAP. I am on PTO tomorrow and on Friday, but I
>>>>> will find time to work at it in the evening if you send me you comments.
>>>
>>> 1) IMO the option should be in the top-level option section, not in a separate
>>> group (use "parser.add_option()").
>>>
>>> Also maybe rename it to --config, AFAIK that's what is usually used.
>>>
>>> A short name ("-c"?) would be nice too.
>>>
>>> Nitpick: if the option is named --config-file, dest should be "config_file",
>>> to make it easier to look it up in the code.
>>>
>>>
>>> 2) Please don't duplicate the knob retrieval code, store knobs in a list and
>>> pass that as an argument to parse_config_file.
>>>
>>>
>>> 3) I'm not sure about using newline as a list separator. I don't know about
>>> other IPA components, but SSSD in particular uses commas, maybe we should be
>>> consistent with that?
>>>
>>>
>>> 4) Booleans should be assignable either True or False, i.e. do not use
>>> _parse_knob to parse them.