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

[Rob Crittenden]

John Dennis wrote:
> On 04/16/2012 01:31 PM, Rob Crittenden wrote:
>> John Dennis wrote:
>>> On 04/13/2012 06:25 AM, Petr Viktorin wrote:
>>>>> When the utility sets logging to console, the extra log message gets
>>>>> printed out there. I agree this isn't optimal.
>>>>> Attached patch removes the console log handler before logging the
>>>>> result.
>>>>>
>>>>
>>>> I read through log_manager, and found that I can do this more cleanly
>>>> with remove_handler.
>>>>
>>>> John, is this a good use of the API?
>>>
>>> The problem you're trying to correct is that under some circumstances a
>>> log message is emitted twice to the console, right?
>>>
>>> Removing the console handler fees like the wrong solution, it's a pretty
>>> big hammer, you have to know when to remove it and once removed any
>>> expectation that messages will appear on the console will be untrue.
>>>
>>> Can you give me a brief explanation as to why you're getting duplicate
>>> messages on the console. I wonder if there isn't a better way to handle
>>> the problem which isn't so invasive and potentially hidden.
>>>
>>> Or is the issue you don't want a console handler at all? If that's the
>>> case then maybe we should provide a configuration that does not create
>>> one, that way it will be explicit and obvious there is no console
>>> handler.
>>>
>>> FWIW, the current configuration of logging is historical dating back to
>>> the beginning of the project. When I added the log manager the intent
>>> was to fix deficiencies in logging, not to modify the existing behavior.
>>> I wasn't clear to me the existing configuration was ideal. If you're
>>> hitting problems because of the existing configuration perhaps we should
>>> look at the historical configuration and ask if it needs to be modified.
>>
>> This patch is standardizing logging the final disposition of a number of
>> commands. Currently is must be inferred.
>>
>> The problem is that some commands have had this disposition added but
>> open no log files so some error messages are being displayed twice and
>> in log format.
>>
>> I think the easiest solution would simply be to scale this back to only
>> those commands that open a log file at all.
>
> You say "command" but command is a loaded word :-) I presume you mean
> command line utility and not an IPA Command object. The reason I'm
> drawing this distinction is because the environment Commands execute in
> are not supposed to change once api.bootstrap() has completed.
>
> Who or what is aggregating the final disposition of a number of
> commands? The reason I ask is because the log_mgr object is global and
> deleting a handler from a global resource to satisfy a local need may
> have unintentional global side effects. How is the aggregation
> accomplished?
>
>

This patch is in the context of the command-line utilities like 
ipa-server-install, ipa-client-install, etc. and not the ipa tool. Some 
of them initialize the api, some do not.

As I said, the intention of my original request was to log the 
disposition of an install command. I think this patch goes too far and 
updates all admin utilities.

rob