On Thu, Aug 09, 2012 at 09:40:36AM +0200, Pavel Březina wrote: > On 08/08/2012 04:36 PM, Stephen Gallagher wrote: > >On Wed, 2012-08-08 at 16:24 +0200, Jakub Hrozek wrote: > >>On Wed, Aug 08, 2012 at 02:23:04PM +0200, Pavel Březina wrote: > >>>This bug was probably introduced with the subdomain patches. The > >>>problem was that sss_dp_get_domains_send() is called even for the local > >>>provider. There are certainly many possible solutions of this issue. I > >>>decided to modify sss_dp_issue_request() to call the callback > >>>immediately if it is issued for local domain. This way, I believe, we > >>>can avoid further problems with issuing request for local domains. > >>I don't think this is the best solution. > >> > >>First, it leaves the check_provider of the per-domain structures we use > >>in responders around while duplicating the same functionality in the DP. > >>At the very least, the patch should remove the "check_provider" tests from > >>the existing loops. > >> > >>What I think would be even better solution is to > >> 1) in the RC just fix the subdomains code so that it shortcuts or > >> doesn't run at all for LOCAL lookups. > >> 2) The work we'll be doing later on #1126 would merge all the loops > >> into one common place. > >> > >>This approach would have the benefit of all the checks being done at one > >>common place, so that we don't forget to add them like we did in the > >>subdomains case and at would be faster at the same time because it > >>wouldn't even contact the DP at all, much like it's done now in the > >>separate loops. > >I think it might be time to more seriously consider decoupling the LOCAL > >provider into a real provider type, instead of having so many > >special-cases throughout the code (i.e. if !local then goto data > >provider) > This patch removes the needs of having special case all around the > code. It just isn't in the scope of this ticket to actually remove > the existing special cases. But if you want to do it in #1126, I'll > just fix the subdomains.
Sure, we're not doing the local provider now. We should just file a ticket: https://fedorahosted.org/sssd/ticket/1469 But even with the current patch, you still need to remove the "check_provider" checks all around the code. They duplicate the same logic as your patch. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
