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

[Petr Viktorin]

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.


-- 
Petr³
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0117-02-Better-logging-for-AdminTool-and-ipa-ldap-updater.patch
Type: text/x-patch
Size: 15663 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130104/85c11d65/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0118-02-Port-ipa-replica-prepare-to-the-admintool-framework.patch
Type: text/x-patch
Size: 45428 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130104/85c11d65/attachment-0001.bin>