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

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

Reply via email to