[Freeipa-devel] [PATCH] 940 apply some validation to some classes only

[Jan Cholasta]

On 20.2.2012 22:56, Rob Crittenden wrote:
> Rob Crittenden wrote:
>> The variable name rdnattr can be misleading. It is only used to give the
>> name of hte RDN in something that can be renamed. Compare this to
>> something like netgroups where the DN has no visible relationship to the
>> content of the object (ipaUniqueId). Only those objects that define
>> rdnattr get a rename option (look at netgroups, for example).
>>
>> rdnattr is always the primary key but not always defined. It should
>> probably be a boolean instead, rdn_is_primary_key or something a bit
>> more obvious. I can make that change here.
>>
>> rob
>
> Updated patch. It seems I broke query a few months ago trying to enforce
> no white spaces in some names.
>
> I did the rdnattr variable rename. Looking back at the changelog this
> was meant to always match the primary key so lets remove the possibility
> that it doesn't. By doing it this way we get the pattern for free.
>
> rob

This is certainly better, but I still have a few concerns:

1. --rename is tied to the RDN change operation. I think this is wrong. 
--rename should be available for all objects, not only when the object's 
RDN attribute and primary key attribute are the same (so that we can 
change the primary key of any object). Similarly, modrdn should be 
triggered for all objects every time the RDN attribute changes, not only 
when the object's RDN attribute and primary key attribute are the same 
(so the DN is properly updated for all objects).

I don't know if this is in the scope of this patch, though.

2. In crud.Create/crud.Update, query is set for params which have the 
ask_create/ask_update flag set. This is an error, as we are obviously 
not querying LDAP with these params (it seems someone forgot to remove 
the query=True keyword argument after copy-pasting from crud.Search).

Honza

-- 
Jan Cholasta