On 04/23/2012 05:19 AM, Petr Viktorin wrote:
This fixes https://fedorahosted.org/freeipa/ticket/2071 (Add final debug
message in installers).
I submitted an earlier version of this patch before (0014), but it was
too much to include in 2.2. Hopefully now there's more space for
restructuring. I think it's better to start a new thread with this approach.
The try/except blocks at the end of installers/management scripts are
replaced by a call to a common function, which includes the final message.
For each specific error, the error handlers in all scripts was almost
the same, but each script handled a different selection of errors.
Instead of having this copy/pasted code (with subtle differences
creeping in over time), this patch consolidates it all in one place.
I like this approach much better than the earlier patch, great, thanks.
I'm a big fan of calling into common code instead of copying code to my
mind the refactoring to utilize common code is great approach. I also
like the fact the logging configuration is not modified after it's
established.
At some point we may want to revist how the log messages are generated.
For example should all communication to the console pass through the
console handler? Is there a logger established for the script? Should
the format of messages emitted to the console be altered? Should all
command line utilities accept the both the verbose and debug flag? Etc.
But for now this is fantastic start in the right direction.
I have not installed and exercised the patch so I can't comment on any
runtime time issues that might be present, but from code inspection only
it has my ACK.
--
John Dennis <jden...@redhat.com>
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
