[Freeipa-devel] [PATCH 0143-0144] different errors/warnings for different LDAP limit type exceeded

[Martin Babinsky]

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.

-- 
Martin^3 Babinsky