Advertising

Some minor compliants.

Ideally, there would be a routine that sets up the logging and handles
command-line arguments in some uniform way (which is also needed before
logging setup to detect ipa-server-install --uninstall).
The original patch did the common logging setup, and I hacked around the
install/uninstall problem too.
I guess I overdid it when I simplified the patch.
I'm somewhat confused about the scope, so bear with me as I clarify what
you mean.

If you abort the installation you get this somewhat unnerving error:
Continue to configure the system with these values? [no]:
ipa : ERROR ipa-server-install failed, SystemExit: Installation aborted
Installation aborted
ipa-ldap-updater is the same:
# ipa-ldap-updater
[2012-03-26T14:53:41Z ipa] <ERROR>: ipa-ldap-updater failed, SystemExit:
IPA is not configured on this system.
IPA is not configured on this system.
and ipa-upgradeconfig
$ ipa-upgradeconfig
[2012-03-26T14:54:05Z ipa] <ERROR>: ipa-upgradeconfig failed,
SystemExit:
You must be root to run this script.
You must be root to run this script.
I'm guessing that the issue is that the log file isn't opened yet.

>

It would be nice if the logging would be confined to just the log.

If I understand you correctly, the code should check if logging has been
configured already, and if not, skip displaying the message?

When uninstalling you get the message 'ipa-server-install successful'.
This is a little odd as well.

ipa-server-install is the name of the command. Wontfix for now, unless
you disagree strongly.

Updated patch: only log if logging has been configured (detected by
looking at the root logger's handlers), and changed the message to “The
ipa-server-install command has succeeded/failed”.

Works much better thanks. Just one request. When you created final_log()
you show less information than you did in earlier patches. It is nice
seeing the SystemExit failure. Can you do something like this (basically
cut-n-pasted from v05)?