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

[Petr Viktorin]

On 01/28/2013 04:36 PM, Petr Viktorin wrote:
> 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.
>

Rebased again.


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