[Freeipa-devel] [PATCHES] 0117-0118 Port ipa-replica-prepare to the admintool framework

[Petr Viktorin]

On 01/04/2013 02:43 PM, Petr Viktorin wrote:
> On 01/03/2013 02:56 PM, John Dennis wrote:
>> On 01/03/2013 08:00 AM, Petr Viktorin wrote:
>>> Hello,
>>>
>>> The first patch implements logging-related changes to the admintool
>>> framework and ipa-ldap-updater (the only thing ported to it so far).
>>> The design document is at http://freeipa.org/page/V3/Logging_and_output
>>>
>>> John, I decided to go ahead and put an explicit "logger" attribute on
>>> the tool class rather than adding debug, info, warn. etc methods
>>> dynamically using log_mgr.get_logger. I believe it's the cleanest
>>> solution.
>>> We had a discussion about this in this thread:
>>> https://www.redhat.com/archives/freeipa-devel/2012-July/msg00223.html; I
>>> didn't get a reaction to my conclusion so I'm letting you know in case
>>> you have more to say.
>>
>> I'm fine with not directly adding the debug, info, warn etc. methods,
>> that practice was historical dating back to the days of Jason. However I
>> do think it's useful to use a named logger and not the global
>> root_logger. I'd prefer we got away from using the root_logger, it's
>> continued existence is historical as well and the idea was over time we
>> would slowly eliminate it's usage. FWIW the log_mgr.get_logger() is
>> still useful for what you want to do.
>>
>>      def get_logger(self, who, bind_logger_names=False)
>>
>> If you don't set bind_logger_names to True (and pass the class instance
>> as who) you won't get the offensive debug, info, etc. methods added to
>> the class instance. But it still does all the other bookeeping.
>>
>> The 'who' in this instance could be either the name of the admin tool or
>> the class instance.
>>
>> Also I'd prefer using the attribute 'log' rather than 'logger'. That
>> would make it consistent with code which does already use get_logger()
>> passing a class instance because it's adds a 'log' attribute which is
>> the logger. Also 'log' is twice as succinct than 'logger' (shorter line
>> lengths).
>>
>> Thus if you do:
>>
>>    log_mgr.get_logger(self)
>>
>> I think you'll get exactly what you want. A logger named for the class
>> and being able to say
>>
>>    self.log.debug()
>>    self.log.error()
>>
>> inside the class.
>>
>> In summary, just drop the True from the get_logger() call.
>>
>
> Thanks! Yes, this works better. Updated patches attached.
>


Here is patch 117 rebased to current master.

-- 
Petr³
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0117-03-Better-logging-for-AdminTool-and-ipa-ldap-updater.patch
Type: text/x-patch
Size: 15729 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130128/42c0a2ca/attachment.bin>