[Freeipa-devel] param type issues

[Petr Viktorin]

On 04/11/2012 07:44 PM, Rob Crittenden wrote:
> John Dennis wrote:
...
>
>> 3) The class name cannot contain an underscore.
>>
>> The find_name() function in makeapi uses a regexp that does not permit
>> an underscore in the name of the class. I presume this was an
>> oversight and not a requirement that class names do not contain
>> underscores.


My interpretation of PEP8 is that class names shouldn't use underscores; 
your class should be named DNParam, not DN_Param.

We already have classes like IA5Str, LDAPError, SSLTransport or 
CLIOptionParser.

find_name() should still be fixed though

>
> Why would you want to? Param classes are by definition simple things:
> Int, Str, etc.
>
>> 4) makeapi is makeing assumptions about dict ordering.
>>
>> makeapi when it generates a string calls repr() on the contents of a
>> dict. It uses that to compare to a previous run to see if they are
>> identical by doing a string comparision. That's not robust. There are
>> no guarantees concerning the ordering of keys in a dict, nor the
>> string values produced by repr(). If you want to compare dicts for
>> equality then you should compare dicts for equality. If you want to
>> use strings for comparison purposes you have to be a lot more careful
>> about how you generate that string representation.
>>
>>
>
> makeapi was rather quick and dirty. dict ordering has always been the
> same (except when it isn't). In the year since we introduced makeapi I
> don't recall a case where dict ordering changed.

A change is slowly coming up.
There is a security issue with predictable hashing (oCERT-2011-003);
the fix for this will make dict ordering randomized between Python runs.
Python 2.6.8 and 2.7.3 (released upstream this week) can do this, but to 
maintain backwards compatibility, it's off by default. Most likely, it 
won't be turned on until Python 3.3.

> I don't want to
> over-engineer things but since we already have dict comparison code in
> the test sutie we can probably leverage that. makeapi is just there to
> keep us honest. It doesn't have the same robustness requirements that
> other code has. In other words if it takes 15 minutes to properly
> compare the dicts then fine. If it is going to take a day then don't
> bother, the bang isn't worth the buck.

Once 2.7.3 is in the distro I recommend setting testing with the 
randomization enabled (export PYTHONHASHSEED=random), as we will want 
IPA to work when users have it on. makeapi will need fixing then.


-- 
Petr³