[Freeipa-devel] Command instantiation

[Petr Viktorin]

On 01/14/2013 06:31 PM, Alexander Bokovoy wrote:
> On Mon, 14 Jan 2013, Jan Cholasta wrote:
>> On 14.1.2013 17:06, Petr Viktorin wrote:
>>>
>>> IPA Command objects sometimes need to pass some data between their
>>> various methods. Currently that's done using the thread-local context.
>>> For an example see dnsrecord_del, which sets a "del_all" flag in
>>> pre_callback and then checks it in execute.
>>>
>>> While that works for now, it's far from best practice. For example, if
>>> some Command can call another Command, we need to carefully check that
>>> the global data isn't overwritten.
>>>
>>>
>>> The other way data is passed around is arguments. The callback methods
>>> take a bunch of arguments that are the same for a particular Command
>>> invocation (ldap, entry_attrs, *keys, **options). By now, there's no
>>> hope of adding a new one, since all the callbacks would need to be
>>> rewritten. (Well, one could add an artificial option, but that's clearly
>>> not a good solution.)
>>> In OOP, this problem is usually solved by putting the data in an object
>>> and passing that around. Or better, putting it in the object the methods
>>> are called on.
>>>
>>> This got me thinking -- why do we not make a new Command instance for
>>> every command invocation? Currently Command objects are only created
>>> once and added to the api, so they can't be used to store per-command
>>> data.
>>> It seems that having `api.Command.user_add` create a new instance would
>>> work better for us. (Of course it's not the best syntax for creating a
>>> new object, but having to change all the calling code would be too
>>> disruptive).
>>> What do you think?
>>>
>>
>> You could extend that to other plugin types as well (e.g. having
>> Object instances with access to a single object's params instead of
>> passing data around in a dict would be superb), but I'm afraid this
>> kind of change won't be easy to do now.


Ah, yes, you've discovered my ultimate goal: rewriting the whole 
framefork :)
But, rewriting the whole framework at once is not realistic, so I'd like 
to do this in little steps. Starting with Commands. That should be possible.

>> The framework was designed around singleton plugin objects right from
>> the beginning. I personally think this design sucks, as every kind of
>> entity in IPA is described by such an object (object classes are
>> singleton objects, methods are singleton objects, etc.), instead of
>> using appropriate Python primitives for the job (object classes should
>> be Python classes, methods should be methods of these classes, etc.).
> One note here. Having method classes separated from the object classes
> allows to add new methods to existing objects without affecting those
> objects and without redefining object classes.
>
> Ability to write simple plugins to extend existing objects is very
> attractive.
>
> In case of methods being methods of Python object classes you'd lose this
> feature and in order to gain it back you would need to deal with
> some sort of method descriptors -- in many cases methods are in fact
> collection of actual procedures working together, for example, most
> of CRUD methods have pre/post modifier callbacks, spanned over elaborate
> Method inheritance tree and allow to amend actual execute code.
> Pulling this into a single Python method would require more work that
> would later grow into an imitation of a separate method class anyway.
>

The difference between functions, methods, and callable objects in 
Python is surprisingly small. I'd bet allowing the implementer to use 
the best tool out out of those for her job would even involve less magic 
than the current approach. (We actually parse the class names with a regex!)


But as I said, this is generalizing my idea too much. I'd like to keep 
my feet on the ground so I'm currently proposing the change for Commands 
only.

-- 
Petr³