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. -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code