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.
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
git://github.com/encukou/freeipa.git
replica-prepare:pviktori-replica-prepare`.
Part of: https://fedorahosted.org/freeipa/ticket/2652
Fixes: https://fedorahosted.org/freeipa/ticket/3285
--
John Dennis <[email protected]>
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel
