On 11/19/2014 01:44 PM, Tomas Babej wrote: > > On 11/19/2014 12:51 PM, Martin Kosek wrote: >> On 11/19/2014 12:41 PM, Tomas Babej wrote: >>> On 11/19/2014 12:24 PM, Martin Kosek wrote: >>>> On 11/19/2014 12:03 PM, Tomas Babej wrote: >>>>> Hi, >>>>> >>>>> When constructing a parent DN in LDAPSearch, we should always >>>>> check that the parent object exists (hence use get_dn_if_exists), >>>>> rather than search on unexistant containers (which can happen >>>>> with get_dn). >>>>> >>>>> Replaces get_dn calls with get_dn_if_exists in *-find commands >>>>> and makes sure proper error message is raised. >>>>> >>>>> https://fedorahosted.org/freeipa/ticket/4659 >>>> Doesn't it produce extra LDAP search thus making all our search commands >>>> slower? Is that what we want? >>> No it does not make all of our LDAP search slower. It only happens for >>> the objects that have parent objects, such as idoverrides or dnsrecords. >> ... and makes them slower. > > What I was pointing out here is that this is not a issue for ALL *-find > commands (e.g user-find, group-find will not suffer from it), as you > incorrectly stated.
I understood that. But it does not make idview-* or dnsrecord-* any faster :-) > >> >>>> Wouldn't it be better to distinguish between LDAP >>>> search with no results and LDAP search with missing parent DN? The reply >>>> looks >>>> different, at least in CLI: >>> Up to discussion. We would probably need to introduce a new exception, >>> like ParentObjectNotFound. >>> >>>> # search result >>>> search: 4 >>>> result: 0 Success >>>> >>>> # search result >>>> search: 4 >>>> result: 32 No such object >>>> matchedDN: cn=accounts,dc=mkosek-f20,dc=test >>>> >>>> Also, I do not think you can just stop using get_dn(), some commands >>>> override >>>> this call to get more complex searches (like host-find searching for >>>> shortname). >>> Look into the get_dn_if_exists, it just wraps around get_dn, so no issue >>> here. Any custom behaviour is preserved. >> Ah, ok, thanks for info. >> >>> To sum up, I think this is worth changing this behaviour by default, >>> ignoring a non-matching value of the parent object is not a correct >>> general approach in my opinion. >> Well, that's the question. Whether we would leave DS to validate the search >> itself or do all the pre-check ourselves. To me, doing just one LDAP search >> and >> processing the error correctly looks better. But I can live even with your >> version then, I will leave the framework guardians like Honza or Petr3 to >> decide. > > I see now what you're trying to suggest. However, the reason boils > down to ipaldap.find_entries method not differentiating between a > LDAP search that returns error code 32 (No such object) and LDAP > search returning error code 0 (Success), but returning no results. > > In both cases errors.NotFound is raised. > > The reason I did not go this way is that changing the find_entries method > is quite more invasive as this is the method subsenqently called by almost > any command. Right. To avoid potential issues, the ParentNotFound would need to be a subclass to NotFoundError so that "except NotFound" still works. _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel