[Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[Petr Viktorin]

On 03/30/2012 11:00 PM, Rob Crittenden wrote:
> Petr Viktorin wrote:
>> On 03/26/2012 05:35 PM, Petr Viktorin wrote:
>>> On 03/26/2012 04:54 PM, Rob Crittenden wrote:
>>>>
>>>> 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)?
>
> diff --git a/ipaserver/install/installutils.py
> b/ipaserver/install/installutils.
> py
> index 851b58d..ca82a1b 100644
> --- a/ipaserver/install/installutils.py
> +++ b/ipaserver/install/installutils.py
> @@ -721,15 +721,15 @@ def script_context(operation_name):
> # Only log if logging was already configured
> # TODO: Do this properly (e.g. configure logging before the try/except)
> if log_mgr.handlers.keys() != ['console']:
> - root_logger.info(template, operation_name)
> + root_logger.info(template)
> try:
> yield
> except BaseException, e:
> if isinstance(e, SystemExit) and (e.code is None or e.code == 0):
> # Not an error after all
> - final_log('The %s command was successful')
> + final_log('The %s command was successful' % operation_name)
> else:
> - final_log('The %s command failed')
> + final_log('%s failed, %s: %s' % (operation_name, type(e).__name__,
> e))
> raise
> else:
> final_log('The %s command was successful')
>
> This looks like:
>
> 2012-03-30T20:56:53Z INFO ipa-dns-install failed, SystemExit:
> DNS is already configured in this IPA server.
>
> rob

Fixed.

-- 
Petr³
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0014-07-Add-final-debug-message-in-installers.patch
Type: text/x-patch
Size: 30375 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120404/65ae5cbf/attachment.bin>