On 01/03/2013 08:00 AM, Petr Viktorin wrote:

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:


I think you'll get exactly what you want. A logger named for the class and being able to say


inside the class.

In summary, just drop the True from the get_logger() call.

The second patch ports ipa-replica-prepare to the framework. (I chose
ipa-replica-prepare because there was a bug filed against its error
handling, something the framework should take care of.)
As far as Git can tell, it's a complete rewrite, so it might be hard to
do a review. I have several smaller patches that make it easier to see
what gets moved where. Please say I'm wrong, but as I understand, broken
commits aren't allowed in the FreeIPA repo so I can only present the
squashed patch for review.
To get the smaller commits, do `git fetch

Part of: https://fedorahosted.org/freeipa/ticket/2652
Fixes: https://fedorahosted.org/freeipa/ticket/3285

John Dennis <jden...@redhat.com>

Looking to carve out IT costs?

Freeipa-devel mailing list

Reply via email to