On 11/21/2014 02:28 PM, David Kupka wrote: > On 11/21/2014 02:12 PM, Tomas Babej wrote: >> >> On 11/21/2014 01:56 PM, David Kupka wrote: >>> On 11/21/2014 01:42 PM, Tomas Babej wrote: >>>> >>>> On 11/21/2014 01:33 PM, David Kupka wrote: >>>>> https://fedorahosted.org/freeipa/ticket/4683 >>>>> >>>>> >>>>> _______________________________________________ >>>>> Freeipa-devel mailing list >>>>> Freeipa-devel@redhat.com >>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel >>>> >>>> - self.read_header() >>>> + try: >>>> + self.read_header() >>>> + except: >>>> + raise admintool.ScriptError('Cannot read backup >>>> metadata') >>>> >>>> >>>> Is the bare except clause really necessary? >>>> >>>> https://docs.python.org/2.7/howto/doanddont.html#except >>>> >>>> >>> Not really. I can't imagine other reaction to any exception raised in >>> read_header() than complain and exit. >>> But you're right that it can hide code errors and make debugging more >>> complicated. >>> Fixed patch attached. >>> >> On another note, I also noticed that read_header leaves leaking file >> descriptor fd. >> >> Can you convert that part to use the with statement? This is a perfect >> opportunity to fix this as you're touching related code. >> > I thought that python takes care of it. Thanks. > Fixed patch attached. >
Works quite nicely. Thanks, ACK. Pushed to: master: 373bbee4e3c25fd6fb41a75b62b09d60da1a5d82 ipa-4-1: b40cf4b283c9df7d960ec80124b45d5361c75320 -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
