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.