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.
rob