[Freeipa-devel] [PATCH] Add group plugin port to new LDAP backend.

[Pavel Zuna]

Jason Gerard DeRose wrote:
> On Thu, 2009-05-14 at 19:06 -0400, Simo Sorce wrote:
>> On Thu, 2009-05-14 at 13:59 -0600, Jason Gerard DeRose wrote:
>>>> When no attributes to retrieve are specified, python-ldap retrieves
>>> them all in 
>>>> the original form - camel case. If we specify them, then it returns
>>> them in the 
>>>> same form as we requested them. The new LDAP backend doesn't use
>>> CIDicts 
>>>> anymore, but only the normal python dict type, so everything is case
>>> sensitive. 
>>>> Of course I can make it return attribute names always as lowercase
>>> if that's 
>>>> what we want.
>>> +1.  I personally think this is the best approach.
>>
>> I would seriously prefer case-insensitive dictionaries unless there is
>> some strong technical/performance reason not to.
> 
> Yes, there is.  We want the processing pipeline to use
> least-common-denominator data types so that it's easy glue in existing
> tools, libraries, extension modules, etc.  Some libraries/extensions
> that expect a dict might fail if passed a CIDict.  And they certainly
> wont know to return our special CIDict.
> 
> I also use the (*args, **kw) calling semantics often in the pipeline,
> which will cause our CIDict to disappear the first time it's passed as
> **keyword arguments.
> 
> And there will be a performance hit in the Python code as I used/abused
> the dict type extensively in order to keep the processing pipeline
> generic and flexible.  The builtin dict type is implemented in C and is
> very fast... but as soon as we subclass it and start overriding methods,
> that performance gets blown out the window big time.  I'd have to do
> some benchmarks, but my shoot from the hip guess is that operations on
> the CIDict are probably in the ballpark of 50 times slower than
> operations on the builtin dict.  Although I personally feel the above
> least-common-denominator issue is a more important reason not to use the
> CIDict.
> 
>> The point is that LDAP *is* case insensitive for attribute names, and we
>> are going to hit bugs (like we did before we had CIDIcts) if we do not
>> acknowledge this fact and try to ignore it or dumb it down.
> 
> There is no reason using lowercase keys will cause bugs.  In fact, it
> will reduce the chances for them.  Direct interaction with LDAP only
> occurs within the Backend.ldap plugin.  All the ldap plugin has to do is
> lowercase the keys of entries it pulls.  By putting this small amount of
> babysitting in the ldap plugin, we prevent unexpected corner cases from
> exploding out into the other 95% of the code.  And we make it much
> easier for existing code-bases to integrate with IPA.  IHMO, this is
> without a doubt the simplest and most maintainable approach.
> 
>> If we are ever going to provide an ldap browser for example it would be
>> strongly desirable to show attributes and values in the same case they
>> were returned by the LDAP server.
> 
> This use case, while interesting, is also an odd ball... IPA, correct me
> if I'm wrong, is largely designed to hide the LDAP details.
> 
> We could always change the Backend.ldap methods to accept a flag to
> preserve the case for special consumers like an LDAP browser.  But
> there's no reason to complicate the rest of the code (which is also the
> great majority of the code) by making this the default, IHMO.
Take a look at the Encoder class I posted on freeipa-devel yesterday. 
It's far from perfect, but it could solves some of these issues. By 
default, 'decode_postprocessor' will be set to 'string.lower()' in the 
LDAP backend and postprocessing will be active for keys only (case of 
values will remain untouched). If a consumer wants to switch 
postprocessing off and get the original case, it can do so before making 
the backend call. The only possible problem I can see at the moment is 
that these settings are effective for the whole backend and probably not 
thread-safe (one plugin changing the setting for all other plugins using 
the backend at the same time).

>> So do we have a strong technical case for why we can't keep using
>> CIDicts ?
> 
> By the way, as far as I know, CIDicts aren't being used at all in the v2
> codebase.  They've never been part of the plugin architecture and I
> don't think they've been used in any of the plugins.
> 
>> Simo.
> 
> So there's my two cents.  Now the question is, did I convenience
> you?  ;)
> 
> -Jason
> 

Pavel