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

[Rob Crittenden]

Jan Cholasta wrote:
> On 29.2.2012 19:45, Rob Crittenden wrote:
>> Jan Cholasta wrote:
>>> 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.
>>
>> Right, not in this scope. An entry where RDN != primary key doesn't need
>> --rename to do a rename, just a mod on whatever its "key" is. We can
>> fake this to have a consistent interface though. Please open a ticket.
>
> Well, you have to do it using --setattr, which is not very user-friendly
> IMO.
>
> https://fedorahosted.org/freeipa/ticket/2466
>
>>
>>>
>>> 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
>>>
>>
>> Updated
>
> ACK.
>
>>
>> rob
>>
>
> Honza
>

Thanks, pushed to master and ipa-2-2

rob