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

2015-08-26 Thread Petr Spacek
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
copypasting 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.

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

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


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

2015-08-26 Thread Jan Cholasta

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




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 

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

2015-08-26 Thread Petr Spacek
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
 copypasting 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 

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

2015-08-26 Thread Jan Cholasta

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
copypasting 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, 

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

2015-08-06 Thread Martin Babinsky

On 08/03/2015 01:56 PM, Martin Babinsky wrote:

On 07/30/2015 08:55 AM, 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



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.


Honza



Attaching updated patch.




Attaching updated patch.

I have also sneaked in a small change that improves the testability of 
installers (making configurator a member of ConfigureTool instance). 
Should I send a separate patch for that?


--
Martin^3 Babinsky
From c73cae8dd0a189a2384f010b81273d88fc450f8d Mon Sep 17 00:00:00 2001
From: Martin Babinsky mbabi...@redhat.com
Date: Wed, 22 Jul 2015 13:55:26 +0200
Subject: [PATCH 1/2] IPA server and replica installers can accept options from
 config file

New option '-c'/'--config' enables ipa-server-install and ipa-replica-install
to obtain parameters from supplied configuration file in INI format.

The file syntax is as follows:
* all options are listed in a single [global] section
* the option name can be derived from long CLI option name by replacing
  dashes with underscores
* to specify a multivalued parameter, assign a list of comma separated
  values to a single option. Escape commas in values like DNs by a
  backslash.

Parameters specified explicitly through CLI options 

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

2015-08-03 Thread Martin Babinsky

On 07/30/2015 08:55 AM, 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



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.


Honza



Attaching updated patch.

--
Martin^3 Babinsky
From c787830e833c96d522b5dfe22bb0a054857a901d Mon Sep 17 00:00:00 2001
From: Martin Babinsky mbabi...@redhat.com
Date: Wed, 22 Jul 2015 13:55:26 +0200
Subject: [PATCH] IPA server and replica installers can accept options from
 config file

New option '-c'/'--config' enables ipa-server-install and ipa-replica-install
to obtain parameters from supplied configuration file in INI format.

The file syntax is as follows:
* all options are listed in a single [global] section
* the option name can be derived from long CLI option name by replacing
  dashes with underscores
* to specify multivalued parameter, assign a list of comma separated values
  to a single option. Whitespace around commas is permitted.

Parameters specified explicitly through CLI options take precedence over the
values contained in config file.

In the case of unknown options present in the config file, the installer will
raise an error listing them.

https://fedorahosted.org/freeipa/ticket/4517
---
 ipapython/install/cli.py | 83 

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

2015-07-30 Thread Jan Cholasta

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



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.



Honza

--
Jan Cholasta

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


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

2015-07-29 Thread Petr Vobornik

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.

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.


If the names are different than cli names, then they should be made 
discoverable somehow or be documented.




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.


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.




--
Petr Vobornik

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


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

2015-07-29 Thread Martin Babinsky

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.


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.


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.


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.


--
Martin^3 Babinsky

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


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

2015-07-29 Thread Petr Vobornik

On 07/29/2015 12:37 PM, Alexander Bokovoy wrote:

On Wed, 29 Jul 2015, Martin Babinsky wrote:

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.

I would prefer that too. Or you can simply allow both _ and -, this
should be relatively simple.


+1




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])?

What about using a different approach -- allowing to specify which
section to process, defaulting to [global]. This would allow to have a
single config file for whole setup, if needed, and just vary which
section to use.


Interesting idea.



Maybe global section could always be processed and the rest could be
used to amend the configuration?

As an example,

[global]
setup_dns
realm = EXAMPLE.COM
domain = example.com
ds-password = SuperSecretPasswordHere
admin-password = EquallySecretPasswordHere
mkhomedir

[m1.example.com]
hostname=m1.example.com


[m2.example.com]
hostname=m2.example.com
setup_dns = False
mkhomedir = False


You can see I also kind of suggest to allow accepting True/Fals to
boolean options to allow _unsetting_ the effect of the default set in
the [global] section.


+1




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'?

Well, there is certain beauty of providing some arguments from the
config file and be asked for the rest. Unattended is more explicit in
the way of handling so I would still keep them separate.



+1

--
Petr Vobornik

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


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

2015-07-29 Thread Jan Cholasta

Dne 29.7.2015 v 12:50 Petr Vobornik napsal(a):

On 07/29/2015 12:37 PM, Alexander Bokovoy wrote:

On Wed, 29 Jul 2015, Martin Babinsky wrote:

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])?

What about using a different approach -- allowing to specify which
section to process, defaulting to [global]. This would allow to have a
single config file for whole setup, if needed, and just vary which
section to use.


Interesting idea.


Maybe, but I don't think it's something that should be in the initial 
implementation, let's keep it simple for now.


--
Jan Cholasta

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


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

2015-07-29 Thread Jan Cholasta

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.




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.




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.




There are probably other issues to discuss. Feel free to write
email/ping me on IRC.



(I haven't looked at the patch yet.)

--
Jan Cholasta

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


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

2015-07-29 Thread Alexander Bokovoy

On Wed, 29 Jul 2015, Martin Babinsky wrote:

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.

I would prefer that too. Or you can simply allow both _ and -, this
should be relatively simple.

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])?

What about using a different approach -- allowing to specify which
section to process, defaulting to [global]. This would allow to have a
single config file for whole setup, if needed, and just vary which
section to use.

Maybe global section could always be processed and the rest could be
used to amend the configuration?

As an example,

[global]
setup_dns
realm = EXAMPLE.COM
domain = example.com
ds-password = SuperSecretPasswordHere
admin-password = EquallySecretPasswordHere
mkhomedir

[m1.example.com]
hostname=m1.example.com


[m2.example.com]
hostname=m2.example.com
setup_dns = False
mkhomedir = False


You can see I also kind of suggest to allow accepting True/Fals to
boolean options to allow _unsetting_ the effect of the default set in
the [global] section.


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'?

Well, there is certain beauty of providing some arguments from the
config file and be asked for the rest. Unattended is more explicit in
the way of handling so I would still keep them separate.

--
/ Alexander Bokovoy

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