[Freeipa-devel] Splitting out ipaldap

[Petr Viktorin]

On 04/14/2015 05:22 PM, Jan Cholasta wrote:
> Hi,
>
> Dne 14.4.2015 v 16:38 Petr Viktorin napsal(a):
>> Hello!
>>
>> As some of you know, I'm looking to help porting FreeIPA to Python 3.
>> One of the major dependencies holding this back is python-ldap, which
>> hasn't been ported yet. Some preliminary porting patches by Raphaël
>> Barrois [0] are ready and have been sent to the python-ldap list. The
>> python-ldap upstream has been very quiet about reviewing them so far,
>> but they're something for me to test against, and maybe improve.
>>
>> To make the testing easier, I'd like to split out "ipaldap" to a
>> stand-alone package, and port it to Python 3 first.
>> This split will make it easier to test (since I don't have to port all
>> of IPA), and being able to use our generic LDAP wrappers in non-IPA
>> projects could maybe also invite some community participation. Also,
>> ipaldap unit tests are somewhat lacking, so I'll help with that.
>> Packaging-wise, I want "ipaldap" to be on the same level as "ipapython"
>> or "ipaserver"; additionally I want to release it on PyPI [1].
>
> Note that I don't consider ipaldap API stable and don't want to put any
> effort in maintaining backward compatibility when something needs to be
> changed, so you might want to hold the PyPI release, or at least put a
> big fat warning in some visible place.

If it's released early & often, it'll at least be visible to interested 
people.
It would be nice to include a roadmap file saying what needs to change 
before we start claiming API stability.

>> My general plan is:
>> - Move ipapython.dn -> ipaldap.dn (keeping ipapython.dn importable for
>> old scripts/plugins)
>
> DNs are not strictly LDAP specific, so I would rather move ipapython.dn
> to a new ipautil package.

I'd rather not, at least until there's something that needs it (and 
doesn't also depend on ipaldap). The scope of "ipautil" is far too badly 
defined for such a package to be useful.

>> - Move CIDict to ipaldap.cidict. For Python 3, I'd really like to
>> replace this with something based on collections.MutableMapping, since
>> the semantics of subclassing "dict" aren't very well defined.
>
> I have WIP which does just that.

Could you send it?

>> - Create a new module for ipaldap-specific exceptions. IPA will use a
>> hook to swap out this module for its own set of exceptions. (Yes, this
>> is a hack, but I'd like to keep ipaldap clean of IPA dependencies. A
>> proper solution will be quite involved, given translatable error
>> messages and XML-RPC numbers.)
>
> I have given this some thought since our last conversation about this
> and I think there won't always be 1-to-1 mapping between ipaldap and
> ipalib errors, so IMO we should go with the usual monkey patching
> approach in ipapython.ipaldap:
>
>      import ipaldap
>      import ipaldap.errors
>      from ipalib import errors
>
>      ipaldap.errors.SomeError = errors.SomeError
>      ipaldap.errors.SomeSimilarError = errors.SomeError
>      ipaldap.errors.SomeOtherError = errors.SomeOtherError
>
>      ...
>
>      try:
>          ipaldap.something()
>      except ipaldap.errors.BaseError as e:
>          # catch ipaldap errors that weren't monkey-patched
>          raise errors.DatabaseError(str(e))

I don't like this idea at all; action-at-a-distance bugs caused by 
monkeypatching are some of the worst bugs to have to deal with.

If someone builds a library on top of ipaldap, and then uses it in a 
project that also uses ipapython, then the library would suddenly start 
raising IPA-specific errors.
If you pass a collection of errors to LDAPClient to use, or even if you 
subclass, or monkeypatch a specific instance, other IPAClient 
instances/subclasses are not affected.

I think limiting the impact of a hack is more preferable than a somewhat 
cleaner solution.

>> - Split ipapython.ipaldap into ipaldap.entry, ipaldap.client,
>> ipaldap.schema_cache; but keep some IPA-specific logic in
>> ipapython.ipaldap (and again keeping all the old names importable).
>
> I don't think schema cache deserves it's own module.

Sure. When I paint that shed, I'll use whatever color looks most 
convenient :)

>> I'll port everything to Python 3 as I move it, but I won't add a py3 run
>> to IPA's test suite, at least until the dependencies are ported. Any
>> breakage on py3 will be mine to fix, for the time being.
>> Speaking of dependencies, one of my goals for this effort is to
>> determine whether replacing python-ldap  by another library, ldap3,
>> would be worth it.
>
> +1 on looking into ldap3

-- 
Petr Viktorin