Menu
▾
▴
python-ldap-dev
|
From: <mi...@st...> - 2002-06-28 07:12:36
Attachments:
result.py
|
HI!
While digging into the performance issues with
ldap.ldapobject.LDAPObject.result() I noticed that the original C
module's result() method does not return results if called with
all=1. See the test script attached.
Here's the output:
*** Using LDAPObject.result() ***
[('', {'objectClass': ['top', 'OpenLDAProotDSE']})]
*** Using C wrapper method directly ***
('RES_SEARCH_RESULT', None)
The RES_SEARCH_RESULT is ok but not None as second tuple item.
I guess nobody noticed that since everybody is solely using the
synchronous methods *_s(). I noticed that quite a while ago but
forgot this unsolved issue.
Since I'm not a C programmer and David is not available anymore
for python-ldap I'd like to ask if someone is willing to dig into
LDAPObject.c's function l_ldap_result(). I guess the
if-else-statement in question is (keep in mind the search
references!):
if (res_type == LDAP_RES_SEARCH_ENTRY
|| res_type == LDAP_RES_SEARCH_REFERENCE
)
pmsg = LDAPmessage_to_python( self->ldap, msg );
else {
int result;
char **refs = NULL;
LDAP_BEGIN_ALLOW_THREADS( self );
ldap_parse_result( self->ldap, msg, &result, NULL, NULL, &refs,
NULL, 0 );
LDAP_END_ALLOW_THREADS( self );
if (result != LDAP_SUCCESS) { /* result error */
char *e, err[1024];
if (result == LDAP_REFERRAL && refs && refs[0]) {
snprintf(err, sizeof(err), "Referral:\n%s", refs[0]);
e = err;
} else
e = "ldap_parse_result";
return LDAPerror( self->ldap, e );
}
pmsg = Py_None;
}
Hope to find a contributor with C knowledge who has some spare
cycles to track that down...
Ciao, Michael.
|
|
From: Derrick 'd. H. <dm...@dm...> - 2002-06-28 20:56:28
|
On Thu, Jun 27, 2002 at 07:41:16PM +0200, Michael Str=F6der wrote:
| While digging into the performance issues with
| ldap.ldapobject.LDAPObject.result() I noticed that the original C
| module's result() method does not return results if called with
| all=3D1. See the test script attached.
| Hope to find a contributor with C knowledge who has some spare
| cycles to track that down...
--- LDAPObject.c.orig Fri Jun 28 15:59:11 2002
+++ LDAPObject.c Fri Jun 28 15:59:37 2002
@@ -818,7 +818,7 @@
e =3D "ldap_parse_result";
return LDAPerror( self->ldap, e );
}
- pmsg =3D Py_None;
+ pmsg =3D LDAPmessage_to_python( self->ldap, msg );
}
=20
result_str =3D LDAPconstant( res_type );
It really does help to not ignore the data that's returned :-).
I haven't thoroughly tested it (actually, I haven't tested any more
than the script you supplied tests it, except that I explicitly added
the 'all' and 'timeout' arguments), but it looks correct and seems to
work.
(the patch is against 2.0.0pre04, but is easy enough to do manually if
CVS has different line numbers)
-D
--=20
"Wipe Info uses hexadecimal values to wipe files. This provides more=20
security than wiping with decimal values." -- Norton SystemWorks 2002 Manual
=20
http://dman.ddts.net/~dman/
|
|
From: <mi...@st...> - 2002-06-29 14:33:39
|
Derrick 'dman' Hudson wrote: > > --- LDAPObject.c.orig Fri Jun 28 15:59:11 2002 > +++ LDAPObject.c Fri Jun 28 15:59:37 2002 > @@ -818,7 +818,7 @@ > e = "ldap_parse_result"; > return LDAPerror( self->ldap, e ); > } > - pmsg = Py_None; > + pmsg = LDAPmessage_to_python( self->ldap, msg ); > } > > result_str = LDAPconstant( res_type ); > > > It really does help to not ignore the data that's returned :-). Oh, yes. > I haven't thoroughly tested it You already noticed the issues with web2ldap. One has to fix two other parts of python-ldap for this patch to work with web2ldap. I've checked in Modules/LDAPObject.c Lib/ldap/ldapobject.py Lib/ldap/async.py. Now the C module's result() is directly wrapped and LDAPObject.c contains the fix above. I guess I will add a derived class for the non-blocking version of LDAPObject.result(). So everyone will be free to choose. Ciao, Michael. |
|
From: Derrick 'd. H. <dm...@dm...> - 2002-06-29 17:41:44
|
On Sat, Jun 29, 2002 at 04:32:52PM +0200, Michael Str=F6der wrote:
| Derrick 'dman' Hudson wrote:
| >- pmsg =3D Py_None;
| >+ pmsg =3D LDAPmessage_to_python( self->ldap, msg );
| >It really does help to not ignore the data that's returned :-).
|=20
| Oh, yes.
|=20
| >I haven't thoroughly tested it
|=20
| You already noticed the issues with web2ldap.
Yep. I was beating myself over the head last night trying to figure
out the right type to return.
| One has to fix two other parts of python-ldap for this patch to work
| with web2ldap.
Ahh.
=20
| I've checked in Modules/LDAPObject.c Lib/ldap/ldapobject.py=20
| Lib/ldap/async.py. Now the C module's result() is directly wrapped=20
| and LDAPObject.c contains the fix above.
I see now :
def search_st(...)
- return self.result(msgid,all=3D1,timeout=3Dtimeout)
+ return self.result(msgid,all=3D1,timeout=3Dtimeout)[1]
=20
That's precisely what I kept running into -- the line
ldap_result[0][1]
_sometimes_ wasn't getting the right element. I thought the result
function wasn't returning the right value (it needed another layer of
list nesting), but when I would add the extra list some other parts
wouldn't work! That certainly explains it :-).
| I guess I will add a derived class for the non-blocking version of=20
| LDAPObject.result(). So everyone will be free to choose.
Isn't that what 'timeout=3D0' is for? I also think the C function needs
to be modified to treat Py_None as NULL to allow python code to
specify a timeout that is indefinitely long.
-D
--=20
"GUIs normally make it simple to accomplish simple actions and
impossible to accomplish complex actions."
--Doug Gwyn (22/Jun/91 in comp.unix.wizards)
=20
http://dman.ddts.net/~dman/
|
|
From: <mi...@st...> - 2002-06-29 17:54:34
|
Derrick 'dman' Hudson wrote:
>
> I see now :
>
> def search_st(...)
> - return self.result(msgid,all=1,timeout=timeout)
> + return self.result(msgid,all=1,timeout=timeout)[1]
>
>
> That's precisely what I kept running into
Please test it! This whole stuff with result() is somewhat
incomplete anyway since the result type
> | I guess I will add a derived class for the non-blocking version of
> | LDAPObject.result(). So everyone will be free to choose.
>
> Isn't that what 'timeout=0' is for?
Yes. But then the application has to handle that. The non-blocking
version of LDAPObject.result() is the one which needed the
time.sleep() hack. I will add it to ldapobject.py in a separate class.
> I also think the C function needs
> to be modified to treat Py_None as NULL to allow python code to
> specify a timeout that is indefinitely long.
IMHO that's done with timeout=-1. Looking at l_ldap_result() in
LDAPObject.c it seems to be implemented correctly (although
reading C makes my eyes hurt):
if (timeout >= 0) {
tvp = &tv;
set_timeval_from_double( tvp, timeout );
} else {
tvp = NULL;
}
Ciao, Michael.
|
|
From: Derrick 'd. H. <dm...@dm...> - 2002-06-29 20:18:57
|
On Sat, Jun 29, 2002 at 07:54:12PM +0200, Michael Str=F6der wrote:
| Derrick 'dman' Hudson wrote:
| >
| >I see now :
| >
| > def search_st(...)
| >- return self.result(msgid,all=3D1,timeout=3Dtimeout)
| >+ return self.result(msgid,all=3D1,timeout=3Dtimeout)[1]
| > =20
| >
| >That's precisely what I kept running into
|=20
| Please test it!
Now I have (some more, at least). I copied async.py from CVS and
backported the patches to LDAPObject.c and ldapobject.py to version
2.0.0pre04 and now web2ldap works great (and faster!). Re-timing the
maillist.py script I originally posted yields this :
real 0m0.206s
user 0m0.070s
sys 0m0.040s
As crude as time(1) may be, that is clearly better and isn't long
enough to measure with my watch or to show up in top :-).
| This whole stuff with result() is somewhat=20
| incomplete anyway since the result type
Looks like your sentence was cut off.
The CVS version of python-ldap doesn't work for me, though. I had to
change some of the sasl stuff in LDAPObject.c for it to compile, and I
get a SEGV with web2ldap. (I also changed a couple other minor
details that gcc complained about) I'll work on that problem later.
=20
| >| I guess I will add a derived class for the non-blocking version of=20
| >| LDAPObject.result(). So everyone will be free to choose.
| >
| >Isn't that what 'timeout=3D0' is for?
|=20
| Yes. But then the application has to handle that.
Isn't that the purpose of not blocking in the first place?
| The non-blocking version of LDAPObject.result() is the one which
| needed the time.sleep() hack.
The version that was in release 2.0.0pre04 was blocking if all was
true. It just did the blocking in python/python-ldap instead of in
C/libldap2.
| I will add it to ldapobject.py in a separate class.
I have no complaints with that.
=20
| >I also think the C function needs
| >to be modified to treat Py_None as NULL to allow python code to
| >specify a timeout that is indefinitely long.
|=20
| IMHO that's done with timeout=3D-1. Looking at l_ldap_result() in=20
| LDAPObject.c it seems to be implemented correctly
Ahh, yeah, that is one suitable convention. Doesn't
timeout=3DNone
sound amusingly accurate though? (in english at least)
-D
--=20
> SELECT * FROM users WHERE clue > 0
0 rows returned
(http://www.thinkgeek.com/images/products/zoom/no-clue.=
jpg)
=20
http://dman.ddts.net/~dman/
|
|
From: <mi...@st...> - 2002-06-30 14:06:01
|
Derrick 'dman' Hudson wrote: > On Sat, Jun 29, 2002 at 07:54:12PM +0200, Michael Str=F6der wrote: > | This whole stuff with result() is somewhat=20 > | incomplete anyway since the result type >=20 > Looks like your sentence was cut off. The result type is not returned in search results when using=20 search_s() which is somewhat incomplete. Search results may=20 contain real entries and search continuations. One has to=20 distinguish that by looking at the type (tuple vs. string). But=20 that's not nice. > The CVS version of python-ldap doesn't work for me, though. I had to > change some of the sasl stuff in LDAPObject.c for it to compile, Hmm, which version of OpenLDAP? I usually compile against CVS=20 version checked out from REL_ENG_2 branch. That works smoothly. Can you post your SASL-related changes? > and I get a SEGV with web2ldap. Did you check http://www.web2ldap.de/faq.html ? -------------------------- snip --------------------------- web2ldap crashs a lot of with segmentation faults. This is not a web2ldap issue. Please check how your Python=20 interpreter was compiled. It has to be build with configure=20 --without-pymalloc. You probably have to rebuild all C extension=20 modules involved afterwards. -------------------------- snip --------------------------- > (I also changed a couple other minor > details that gcc complained about) Again: Post these changes for open discussion. > | Yes. But then the application has to handle that. >=20 > Isn't that the purpose of not blocking in the first place? >=20 > | The non-blocking version of LDAPObject.result() is the one which > | needed the time.sleep() hack. >=20 > The version that was in release 2.0.0pre04 was blocking if all was > true. It just did the blocking in python/python-ldap instead of in > C/libldap2. Yes. My wording was rather misleading. Blocking yes, but not=20 locking all the time. Therefore other threads calling into the=20 LDAP libs are allowed to run although one thread is blocked. > | >I also think the C function needs > | >to be modified to treat Py_None as NULL to allow python code to > | >specify a timeout that is indefinitely long. > |=20 > | IMHO that's done with timeout=3D-1. Looking at l_ldap_result() in=20 > | LDAPObject.c it seems to be implemented correctly >=20 > Ahh, yeah, that is one suitable convention. Doesn't > timeout=3DNone > sound amusingly accurate though? (in english at least) Well, I have no clue why David chose -1 instead of None. Ciao, Michael. |
|
From: David L. <dav...@it...> - 2002-06-30 23:42:25
|
On Sun, 30 Jun 2002, Michael Str=F6der typed thusly: > > | >I also think the C function needs > > | >to be modified to treat Py_None as NULL to allow python code to > > | >specify a timeout that is indefinitely long. > > | > > | IMHO that's done with timeout=3D-1. Looking at l_ldap_result() in > > | LDAPObject.c it seems to be implemented correctly > > > > Ahh, yeah, that is one suitable convention. Doesn't > > timeout=3DNone > > sound amusingly accurate though? (in english at least) > > Well, I have no clue why David chose -1 instead of None. that's the C/unix convention. however, looking at the documentation of select.select() it seems that using None could be better justified select(rlist, wlist, xlist[, timeout]) -> (rlist, wlist, xlist) [...] The optional 4th argument specifies a timeout in seconds; it may be a floating point number to specify fractions of seconds. If it is absent or None, the call will never time out. d --=20 David Leonard Dav...@it... Dept of Inf. Tech. and Elec. Engg _ Ph:+61 404 844 850 The University of Queensland |+| http://www.itee.uq.edu.au/~leonard/ QLD 4072 AUSTRALIA ~` '~ B73CD65FBEF4C089B79A8EBADF1A932F13E= A0FC8 |
|
From: <mi...@st...> - 2002-07-01 00:33:31
|
Derrick 'dman' Hudson wrote: > > - pmsg = Py_None; > + pmsg = LDAPmessage_to_python( self->ldap, msg ); > > It really does help to not ignore the data that's returned :-). There's still something wrong with that when a search continuation is returned. It simply does not return the last empty search result which terminates the while loop. Strange enough it works from web2ldap but not from a simple demo using ldap.async. Hmm... Ciao, Michael. |
Thanks for helping keep SourceForge clean.
X