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

Reply via email to