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.
rob
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?
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. ;)
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
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
On ke, 09 kesä 2021, Rob Crittenden via FreeIPA-devel wrote:
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.
Frankly, that's was the only point I was able to come with. Definitely should log the warning/suggestion but other than this, I have no idea what else can we add.
freeipa-devel@lists.fedorahosted.org
-
Alexander Bokovoy -
Rob Crittenden