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

[Martin Basti]

On 13.04.2016 16:48, Petr Spacek wrote:
> 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!
>
pushed to master:
* 1f0959735f9828a09439f17f1468dcd3dfb914db differentiate between limit 
types when LDAP search exceeds configured limits
* 62bb478e112cd4677e681f4750c5f5e5c9221607 specify type of exceeded 
limit when warning about truncated search results