On 8.3.2013 14:14, Petr Viktorin wrote:
On 03/07/2013 05:42 PM, Jan Cholasta wrote:
Patch 191:

The patch is missing the ipapython/ipaldap.py file.

On 7.3.2013 18:29, Petr Viktorin wrote:
> It's there, it's just copied from ipaserver/ipaldap.py with a small
> change at the bottom.

There is no sign of the file, except in the patch header and the patch cannot be applied with git am nor with git apply. But perhaps I'm doing something wrong.


I think it should go into ipalib instead of ipapython. <rant> It doesn't
make sense to keep ipapython and ipalib separate if they depend on each
other. We should either merge them or clean up the mess by removing
ipalib imports from ipapython. I'm not saying we should do it now, just
please don't add new modules to ipapython which import from ipalib.
</rant>

This is a bigger problem.
Conceptually ipaldap should be in ipapython, it just needs the
problematic errors and text modules. I think we should move those rather
than ipaldap.

Moving test to ipapython makes sense. I think we should have a separate set of errors for ipaldap and translate them to framework errors in ipalib.


Also I am not very fond of the "ipa" prefix in "ipaldap". The module
lives in the namespace of our own package, so there's no need for it to
have such a prefix, is there?

It's nice to have a unique name for talking about it.
Since "ldap" is already a package we use, having another "ldap" would be
confusing, even if it is properly namespaced.

Since ipaldap should be the sole user of python-ldap after the refactoring is done, I don't think there would be any confusion. But whatever, I'm fine with both.


Patch 193:

+            scope=conn.SCOPE_BASE,
+            filter='objectclass=pkiCA',
+            attrs_list=[ca_cert_attr],

Can we use a proper filter here please?

+    :param conn: Bound LDAPConnection that will be used for searching

Fixed, thanks

LDAPClient

Patch 194:

-                ldap.set_option(ldap.OPT_X_TLS_REQUIRE_CERT, True)

and

-                lh.set_option(ldap.OPT_X_TLS_DEMAND, True)

Is removing these options safe?

I re-added them.

I also removed the forgotten debugging `raise`s Martin found.

Adding patch 0196, which disables downloading the schema for discovery.

Updated patches attached. They now depend on Honza's patches 116-119


ACK.

Honza

--
Jan Cholasta

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to