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Ā³

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

Reply via email to