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 <jden...@redhat.com>

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

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

Reply via email to