Martin Kosek wrote:
On 08/08/2012 11:02 PM, John Dennis wrote:
All the issues Martin discovered (except for the ip-address parameter) are now
fixed and pushed to the dn repo. Also now the dn repo is fully rebased against
master (except for one commit for ticket 2954 which I had to revert, see ticket
for details).

Thank you for the continued testing.

FYI: to use the dn repo:

git clone git://fedorapeople.org/~jdennis/freeipa.dn.git
git checkout dn


Good. I see that all issues except the ipa-replica-prepare are now fixed. I
verified that this is indeed an issue caused by the DN refactoring, I am
attaching a patch fixing the issue. With this patch, ipa-replica-prepare issue
disappears.

Petr Vobornik also noticed an issue with trust-show command. I am attaching a
patch with fix as well. We push the patches when we are pushing your work.

I have not found any more show-stopping issues, so I will just continue my
testing, and also give space to other testers in case they discover something 
else.

Martin

I've been going through the diffs and have some questions in ldap2.py, these are primarily pedantic:

What is the significance of L here:

'2.16.840.1.113730.3.8.L.8'      : DN,  # ipaTemplateRef

There are quite a few assert isinstance(dn, DN). I assume this is mainly meant for developers, we aren't expecting to handle that gracefully at runtime?

It seems to me that allowing a DN passed in as a string would be nice in some cases. Calling bind, for example, looks very awkward and isn't as readable as cn=directory manager, IMHO. What is the downside of having a converter for these?

search_ext() has an extra embedded line which makes the function a bit confusing to read.

Do we need to keep _debug_log_ldap?

In search_ext_s() (and a few others) should we just return self.convert_result() directly?

In ldap2::__init__ what would raise an AttributeError and would we want to hide that fact with an empty base_dn? Is this attempting to allow the use of ldap2.ldap2 in cases where the api is not initialized?

In ipaserver/ipaldap.py there is a continuance of the assertions, some of which don't seem to be needed. For example, in toDict() it shouldn't be possible to create an Entry object without having self.dn be a DN object, so is this assertion necessary?

rob

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

Reply via email to