Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why everything is decoded to unicode instead of handling as bytestrings? #16

Closed
datakurre opened this issue Oct 20, 2016 · 5 comments
Closed

Comments

@datakurre
Copy link
Contributor

datakurre commented Oct 20, 2016

It looked like a good idea that node.ext.ldap makes sure that all incoming bytestrings are decoded into unicode, but then I got profiling from Zope:

17040562 function calls (15158924 primitive calls) in 12.329 seconds
...
ncalls  tottime  percall  cumtime  percall filename:lineno(function)
1426700/40161    1.187    0.000    4.255    0.000 utils.py:180(decode)

(That was from listing a group of 100 users, after spending a few days in optimizing LDAP queries in node.ext.ldap and switching bda.cache to use pylibmc instead of python-memcached. The called decode is node.utils:decode.)

How insane it would be to handle all strings as bytestrings (assuming utf-8 as defined by LDAP spec) and handle decoding on the layer on top of node.ext.ldap?

I assume that this is WONTFIX, because it would break any app using node.ext.ldap.

Does the dependencies of node.ext.ldap assume require unicode strings, or would it be enough for me to fork node.ext.ldap and pas.plugins.ldap to only do encoding/decoding in pas.plugins.ldap?

@rnixx
Copy link
Member

rnixx commented Oct 22, 2016

It would be possible to fix this by defining a config flag whether to encode/decode transparently in node.ext.ldap or not.
Therefor dedicated encode/decode functions are needed considering the config flag (which may even be global)

@datakurre
Copy link
Contributor Author

Agreed. Actually, it was not so hard *) after all to remove all encoding/decoding, so adding flag instead should be doable.

*) datakurre@2328007

rnixx referenced this issue in datakurre/node.ext.ldap Oct 23, 2016
@datakurre
Copy link
Contributor Author

@rnixx What would be a proper place in node.ext.ldap for a such flag?

@rnixx
Copy link
Member

rnixx commented Oct 24, 2016

@rnixx
Copy link
Member

rnixx commented Apr 16, 2019

python-ldap in python 3 by default handles everything but attribute values as unicode. Theres a B/C mode for python 2 which is is used in latest master now.

@rnixx rnixx closed this as completed Apr 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants