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<[email protected]> > > > > 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 _______________________________________________ Freeipa-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/freeipa-devel
