Rob Crittenden via FreeIPA-devel wrote:
Alexander Bokovoy wrote:
> On ti, 13 huhti 2021, Rob Crittenden via FreeIPA-devel wrote:
>> Currently the framework has four types of methods:
>>
>> - pre_callback: called before any real work is done. Usually used to
>> tune up incoming options
>> - execute: does the brute work. For most CRUD plugins this is all done
>> in baseldap.py in the LDAP* classes
>> - post_callback: supposed to be data cleanup for presentation to the
>> caller but in some cases actual work is being done. user-add calls
>> ldap.add_entry_to_group for example.
>> - exc_callback: special case handling for exceptions
>>
>> If you have "work" to be done that would normally be done in execute
>> there is no way to do it other than to abuse post_callback to get it
>> done.
>>
>> Overriding execute is only an option if you want to completely replace
>> the call, otherwise order execution is going to be strange.
>>
>> Say for example in user_add you want to do work after the execute() is
>> done so you'd do something like:
>>
>> class user_add()
>> def execute(stuff):
>> result = super(user_add, self).execute(stuff)
>> # some extra work
>> return result
>>
>> The order of execution is:
>>
>> user_add:pre_callback
>> user_add:execute()
>> LDAPCreate:execute()
>> user_add:post_callback()
>> user_add:execute()
>>
>> This is because the LDAP* baseldap methods handle the pre/post/exc
>> callbacks directly. So in order to be able to insert more "work" in
>> execute itself we'd need another callback type.
>>
>> Why do I want this?
>>
>> One feature of post_callback is that for add/mod methods
>> ldap.get_entry() is called so we can prepare the final results to be
>> returned to the user for display.
>>
>> There are cases where the user doesn't need or case to see that result.
>> We could save an least one LDAP query by returning only errors in this
>> case.
>>
>> The use case is mostly around bulk operations where the user doesn't
>> care about seeing the UID of all users added, for example.
>>
>> To do this I think we can skip post_callback altogether, assuming it
>> does no real "work".
>>
>> In the case of user_add/mod skipping post_callback would save three
>> queries: fetch the user, has_keytab and has_password.
>>
>> But before re-architecting this I'd like to get some opinions on it.
>>
>> I can do a full-blown design if desired.
>
> So your proposal is multi-fold:
>
> - add a callback type that can be used to perform operations that could
> always be done as a part of execute() after the main processing of
> add/mod was done but before post_callbacks were to be called
>
> - use the result of executing new type callbacks to decide whether to
> do or skip post_callback() execution
>
> - move content of post_callback that does not depend on the
> availability of the final results into this new callback type
>
> - leave in the post_callback() state only those operations that rely on
> the final results
>
> Is this right?
Mostly. I don't expect to skip any execute callbacks but you do raise an
interesting point. What gets passed into and out of each result callback
is still TBD. I think that the baseldap/top-level execute should create
the final result and dn, entry_attrs, etc will be passed around.
> Assuming it is right, the only issue I have is an impact to third-party
> plugins. We can communicate such change of course but it is going to
> break someone's plugin, for sure. So a better documentation would be
> needed. ;)
>
Good point. I wonder if we can set a context variable for when in
post_callback that gets used in any baseldap method that does a write so
that an exception is raised. That would certainly get admins attention.
It would also be a bit harsh. We could log something instead but that
would rely on admins to read the logs.
Ping. Any additional feedback would be great.
rob