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

Wed Aug 26 10:44:16 UTC 2015

On 26.8.2015 12:00, Petr Spacek wrote:
> 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.

User interface is not necessarily only CLI and config files and I would 
prefer not to mutilate the user interface in general with CLI specifics. 
If you want 100% CLI compatibility you can do the following and don't 
bother with any new code in IPA at all:

     $ echo --no-ntp >options
     $ echo --forwarder 1.2.3.4 >>options
     $ echo --forwarder 5.6.7.8 >>options
     $ ipa-server-install $(cat options)

Interface consistency is important in any case, and providing it in one 
place just to sacrifice it in other place does not really improve anything.

>
> 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.

-- 
Jan Cholasta