[Freeipa-devel] param type issues

[John Dennis]

On 04/11/2012 01:44 PM, Rob Crittenden wrote:
> John Dennis wrote:

 >> The default value should be treated just as if the user had supplied
 >> the value passing through all the same normization and conversion steps.

 > Params have always been rather primitive types so this has never come
 > up. It seems like a lot of extra work to run every default through a
 > validator but I don't think that's a show-stopper.

It's not much extra work, default values aren't evaluated that often, 
the generality is useful and the framework tries otherwise to be 
symmetric and general.

Plus if you ever look at how much code we execute via the framework to 
do the simplest of things it would be somewhat shocking :-)


>> 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.
>
> Why would you want to? Param classes are by definition simple things:
> Int, Str, etc.

Simple example, I originally named the DN parameter DN following your 
intuitive suggestion. But we already have a type named DN, thus it's a 
name conflict. So, the parameter type had to be called something other 
than DN. Also, the existing parameter types don't really capture the 
fact they can only ever be used as parameter to a command (e.g. Str is 
very generic name, but I digress). So I decided to pick a name that both 
captured the fact it's a DN type *and* it can only be used in the 
context of a command parameter, DN_Param seemed to make sense, but I 
discovered makeapi silently stumbled on this and failed to recognize it 
all. It just seems like not permitting a type name to have an underscore 
is an arbitrary limitation, but I'm really not hung up over the name, 
rather it was a time spent to figure out why makeapi was failing that 
was the real issue.

>> 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. 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.

I agree, I don't think this deserves recoding at this point, it's worked 
so far. It was more to capture the issue, I had to take a hard look at 
how makeapi worked and it was just something I noticed while debugging.


-- 
John Dennis <jdennis at redhat.com>

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/