On 8.4.2016 17:13, Martin Babinsky wrote:
> On 04/01/2016 05:37 PM, Martin Babinsky wrote:
>> On 03/24/2016 04:14 PM, Martin Babinsky wrote:
>>> On 03/22/2016 04:28 PM, Rob Crittenden wrote:
>>>> Martin Babinsky wrote:
>>>>> On 03/21/2016 12:25 PM, Jan Cholasta wrote:
>>>>>> On 21.3.2016 10:17, Petr Spacek wrote:
>>>>>>> On 18.3.2016 13:49, Rob Crittenden wrote:
>>>>>>>> Martin Babinsky wrote:
>>>>>>>>> These patches implement behavior agreed upon during discussion of
>>>>>>>>> https://fedorahosted.org/freeipa/ticket/5677
>>>>>>>>> However I'm not sure if we want to push them into 4-3 branch (the
>>>>>>>>> ticket
>>>>>>>>> is triaged into 4.3.2 milestone) since they modify the framework
>>>>>>>>> behavior quite a bit.
>>>>>>>>> If there is no need to have it there (CC'ing Milan since he is the
>>>>>>>>> reporter), I would retriage it into 4.4 milestone.
>>>>>>>> + desc="while getting entries (search base: '{}',"
>>>>>>>> + "filter: {})".format(base_dn, filter))
>>>>>>>> This is going to expose parts of the DIT in an error message to
>>>>>>>> users. We have
>>>>>>>> tried in the past to hide the implementation. I'd propose logging
>>>>>>>> the
>>>>>>>> error
>>>>>>>> and making the exception less verbose.
>>>>>> I agree with Rob here, we shouldn't expose internal stuff in error
>>>>>> messages for users.
>>>>>> In this particular case, even if we included internal stuff in the
>>>>>> error
>>>>>> message, it should be the error message returned by the server rather
>>>>>> than this ad-hoc message.
>>>>>>> IMHO it actually helps to print the DN. At very least the user can
>>>>>>> see
>>>>>>> if the
>>>>>>> error is happening always with the same DN or if it keeps changing.
>>>>>>> In other words, for user it is not that important to understand
>>>>>>> meaning of the
>>>>>>> DN but it might be important to see if it is the same or not.
>>>>>> I can't imagine a situation where it would actually be useful for the
>>>>>> user (as opposed to the admin, who has access to logs) to know the
>>>>>> base
>>>>>> DN of some arbitrary LDAP search operation. Could you give an example?
>>>>> Right, attaching updated patches.
>>>> I may have suggested debug logging the detailed error. I was wrong. This
>>>> should log at the error level so it always appears in the logs. This may
>>>> be a spurious error and having the user turn on debug logging to capture
>>>> the reasons would be asking a lot.
>>>> rob
>>> That's right, the user would then have to enable debug mode and re-run
>>> the command. I have changed the log level to error as you suggested.
>> Bump for review.
> Moar bumps.

ACK, sorry for the delay!

Petr^2 Spacek

Manage your subscription for the Freeipa-devel mailing list:
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to