Dne 2.11.2011 16:11, Alexander Bokovoy napsal(a):
On Wed, 02 Nov 2011, Jan Cholasta wrote:
I see a number of problems with the patch:

   * Only Command subclasses are finalized (that might be the reason
why it is so fast)
   * You assume that the first operation on a plugin instance is a
call on the args/options/params namespaces, but there is no
guarantee that it will be
   * The whole approach is error-prone, as it requires the programmer
to carefully setup the attributes so that the plugin can be properly
finalized (this should be done automatically, so that every plugin
is guaranteed to be finalized without complicated setup)
Perhaps you need to look at why finalize() is there at all. We have
certain properties that are set only during finalize() method
execution. If something cannot be performed without accessing those
properties, it cannot be done before finalize() is performed.
Anything else is allowed. So it boils down to being able to intercept
the access to those properties that change their value in effect of
finalize() call.

What this all means is if a plugin writer has something in mind
when changing properties in his/her re-implementation of finalize()
method, it is simple to mark up those properties as such and this
patch gives you a simple process and means to do so.

Callable instances are a consequence of the above --
Command.__call__() does use properties that are changed due to
finalize() being run. In fact, there are so many places in FreeIPA
where we simply expect that foo.args/params/output_params/output is
both iterable and callable instance that it is not even funny.

Now, the process established by the proposed patch is actually
dependent only on the fact that bound instance has finalize() method.
This is quite generic and can be used for all other types of objects.


The real problem is in the API class so it should be fixed there, not worked around in Plugin subclasses.

--
Jan Cholasta

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to