On 11/10/2015 04:43 PM, Martin Babinsky wrote: > On 11/10/2015 04:35 PM, Tomas Babej wrote: >> >> >> On 11/10/2015 03:35 PM, Martin Babinsky wrote: >>> On 10/27/2015 04:24 PM, Tomas Babej wrote: >>>> Hi, >>>> >>>> this couple of patches harden the adtrust installer. >>>> >>>> Details in the commit messages. >>>> >>>> Fixes: https://fedorahosted.org/freeipa/ticket/5134 >>>> >>>> Tomas >>>> >>>> >>>> >>> NACK, >>> >>> in the first patch you forgot to instantiate the caught exception in the >>> following snippet: >>> >>> + except Exception: >>> + root_logger.debug("Exception occured during SID generation: >>> {0}" >>> + .format(str(e))) >>> >>> You should use 'except Exception as e:'. >>> >>> I'm also not quite sure that it is enough to log the error at debug >>> level. >>> >>> If the sidgen task somehow fails, isn't it something which should >>> interest the user and deserve at least warning-level message? >>> >> >> Thanks for catching this. Inappropriate message level indeed, I probably >> wasn't using my brain much when writing that snippet :) >> >> Updated patchset attached. >> >> Tomas >> > Thanks, ACK. >
Pushed to master: 767b8de01298e9de20f3b85bad5ac98472b14e99 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code