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

[Jason Gerard DeRose]

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.

> 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