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

[Martin Babinsky]

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.

-- 
Martin^3 Babinsky