[Freeipa-devel] [PATCH 48/48] Ticket #1879 - IPAdmin undefined anonymous parameter lists

[Martin Kosek]

On Tue, 2011-10-04 at 14:26 +0200, Jan Cholasta wrote:
> On 26.9.2011 21:52, John Dennis wrote:
> > The IPAdmin class in ipaserver/ipaldap.py has methods with anonymous
> > undefined parameter lists.
> >
> > For example:
> >
> >      def getList(self,*args):
> >
> > In Python syntax this means you can call getList with any positional
> > parameter list you want.
> >
> > This is bad because:
> >
> > 1) It's not true, *args gets passed to an ldap function with a well
> > defined parameter list, so you really do have to call it with a
> > defined parameter list. *args will let you pass anything, but once it
> > gets passed to the ldap function it will blow up if the parameters do
> > not match (what parameters are those you're wondering? see item 2).
> >
> > 2) The programmer does not know what the valid parameters are unless
> > they are defined in the formal parameter list.
> >
> > 3) Without a formal parameter list automatic documentation generators
> > cannot produce API documentation (see item 2)
> >
> > 4) The Python interpreter cannot validate the parameters being passed
> > because there is no formal parameter list. Note, Python does not
> > validate the type of parameters, but it does validate the correct
> > number of postitional parameters are passed and only defined keyword
> > parameters are passed. Bypassing the language support facilities leads
> > to programming errors.
> >
> > 5) Without a formal parameter list program checkers such as pylint
> > cannot validate the program which leads to progamming errors.
> >
> > 6) Without a formal parameter list which includes default keyword
> > parameters it's not possible to use keyword arguments nor to know what
> > their default values are (see item 2). One is forced to pass a keyword
> > argument as a positional argument, plus you must then pass every
> > keyword argument between the end of the positional argument list and
> > keyword arg of interest even of the other keyword arguments are not of
> > interest. This also demands you know what the default value of the
> > intermediate keyword arguments are (see item 2) and hope they don't
> > change.
> >
> > Also the *args anonymous tuple get passed into the error handling code
> > so it can report what the called values were. But because the tuple is
> > anonymous the error handler cannot not describe what it was passed. In
> > addition the error handling code makes assumptions about the possible
> > contents of the anonymous tuple based on current practice instead of
> > actual defined values. Things like "if the number of items in the
> > tuple is 2 or less then the first tuple item must be a dn
> > (Distinguished Name)" or "if the number of items in the tuple is
> > greater than 2 then the 3rd item must be an ldap search filter". These
> > are constructs which are not robust and will fail at some point in the
> > future.
> >
> > This patch also fixes the use of IPAdmin.addEntry(). It was sometimes
> > being called with (dn, modlist), sometimes a Entry object, or
> > sometimes a Entity object. Now it's always called with either a Entry
> > or Entity object and IPAdmin.addEntry() validates the type of the
> > parameter passed.
> >
> > --
> > John Dennis<jdennis at redhat.com>
> >
> > Looking to carve out IT costs?
> > www.redhat.com/carveoutcosts/
> >
> 
> ACK.
> 
> Honza
> 

Pushed to master (additional information in a thread for Alexander's
patch 0037).

Martin