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. > >>> 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. > Martin -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel