Draft: new response handling API #464
Conversation
Modules/LDAPObject.c
Outdated
| .name = "_RawLDAPMessage", | ||
| .doc = "LDAP Message returned from native code", | ||
| .fields = message_fields, | ||
| .n_in_sequence = sizeof(message_fields)/sizeof(*message_fields) - 1, |
There was a problem hiding this comment.
Set this to 2 to preserve backwards compatibility in result. (And ideally, at the same time, deprecate using result() as a tuple, along with resultN, in favor of the named fields).
This would allow old code to continue using type, data = result().
Setting this to a value that will grow as more fields are added defeats the purpose of PyStructSequence – you might as well use a regular object with getset descriptors.
There was a problem hiding this comment.
This doesn't feel remotely compatible with LDAPObject.result() anyway, that's one of the reasons the new API is split into a Connection object (which gets a better request API too in a future PR). Not sure whether to make the result* APIs a (heavyweight) wrapper for this or keep it for now.
Keeping n_in_sequence automatic is an oversight from initial development, thanks for pointing that out, I'll fix it in the next version. What is your take on the Response subclassing logic? I will probably have to replicate it for ExtendedResponse and intermediates, can you see any conceptual issues with how it is written/is it a total hack?
There was a problem hiding this comment.
Sadly I won't have time to review the whole PR :(
Could you point to the part I should look at?
There was a problem hiding this comment.
Turns out I forgot to add the Python files into the commit so my last comment probably didn't make much sense. I meant the Python bits, this in particular.
There was a problem hiding this comment.
At first glance, that looks more complicated than it needs to be. i don't see much advantage over setting the missing attributes to None (or keeping them missing). Is there an advantage?
There was a problem hiding this comment.
The subclassing allows one to expose response-specific methods (Compare result, allow parsing the extended result) and if the user is on 3.10, allow pattern matching, on others, make the code more readable, especially if controls were subjected to the same treatment.
mistotebe
force-pushed
the
response_api
branch
5 times, most recently
from
abb123e to
756fba2
Compare
mistotebe
force-pushed
the
response_api
branch
from
a35462a to
90652b1
Compare
mistotebe
force-pushed
the
response_api
branch
3 times, most recently
from
cedc9c3 to
6b8f56e
Compare
mistotebe
force-pushed
the
response_api
branch
from
6b8f56e to
12c3b61
Compare
|
@mistotebe I'm thinking about python-ldap 4.0 release. |
|
Any feedback is welcome, yes. I've been using this branch elsewhere with some success and the tools added here certainly make porting some of my tools (e.g. syncmonitor) from python-ldap0 an to some extent pldap easier. Don't think I'll be abandoning pldap any time soon (there is no replacement to its stub server implementation for protocol level testing) but definitely closer to dropping python-ldap0 after we get something like this in. There's still a little bit of a kludge in how e.g. searches are represented, so the API might still evolve, and it's only a stepping stone towards:
But it's definitely useful already so happy to start cleaning up or breaking it up starting with bits you consider less contentious and go from there. |
Maybe my project is suitable for this. I am supporting a nice user-friendly API with sync and async, hiding the |
You should definitely be able to make use of the new interfaces and response classes added here to make your own life easier. In terms of request API (and async), yes yours looks somewhat similar to what I wanted, but there is one important difference: any request (not just search) can return any amount of "intermediate" responses (search entry, referral entry, intermediate) and then a final response or exception so a request object should (always?) be iterable. A caller might be able to say they (don't) expect those, they still have a choice to let the whole request finish or they can receive messages one by one and pattern match on the type (SearchResultEntry, SyncInfoNewCookie, generic IntermediateResponse, ...) etc. There is a partial draft of how the async side could look, what you get is a Future and optionally an iterable for the intermediate responses. We can definitely see how to converge those two APIs because in some ways your take is close to what I had in mind and a lot of things I can see there should simplify to "syntactic sugar"/shorthand over the new API. |
|
Thanks for the explanation. |
Slower? Are we both talking about ldap_result() with
I mean SearchReference, not Result.result == LDAP_REFERRAL. Opting to handle referrals is an option, sure, but the client should have an option to return them (and sometimes that makes sense?).
Often syncrepl control (mode=refreshAndPersist) is the easiest one, or LDAP TXN (RFC 5805). Both exist in OpenLDAP, syncrepl needs the syncprov overlay configured, TXNs need a compatible backend (e.g. back-mdb). |
Uh oh!
There was an error while loading. Please reload this page.