On 10/03/2016 03:15 PM, Jakub Hrozek wrote:
On Mon, Oct 03, 2016 at 02:07:25PM +0200, Pavel Březina wrote:
> On 10/03/2016 12:27 PM, Jakub Hrozek wrote:
>> Hi,
>>
>> we might be able to do some refactoring in the next couple of weeks
>> prior to working on the next release.
>>
>> I wrote up a proposal here:
>>
https://fedorahosted.org/sssd/wiki/DesignDocs/OneFifteenCodeRefactoring
>> for your convenience, I copied the contents inline as well.
>
>> === Use the shared `cache_req` module for responder look-ups ===
>
> Already in progress, some responders are done. I'm currently working on NSS.
> This is a must have and doable in the scope.
>
>> === Refactor group lookups for better performance ===
>
> Ack.
>
>> === Refactor the sdap_id_ops.c module ===
>
> The sdap_id_ops calls should at least be moved into low level ldap code and
> turned to one-return-code-only api. We often forget to use it or we use it
> incorrectly.
Yes, this part I think is doable in the four-weeks time. The other thing
we talked about (wrapping the searches in a request that takes care of
everything and optionally just returns a single error), probably not..
>
>> === Provide a way to pass intermediate data between requests ===
>
> This can be probably done nice and clean with our new DP interface. Just
> fire a new request with new data.
So the DP interface would return sysdb_attrs* or a similar data type?
>
>> === Upstream the PoC tests that utilize Samba AD for AD provider testing ===
>
> For me, higher priority is to make CI test more useful when debugging.
Fabiano did a lot of work there, what is missing in your opinion?
>
> I added two new sections: SBUS Improvements
Can you give me (or add to the page :-)) an example of what is wrong
with the current sbus code? I don't disagree per se, I guess I don't see
the issue, though..
From the top of my mind, there are two main issues:
1) handler return EOK if reply to the request was send or is in
asynchronous processing, so there are several places in the code where
the handler can be finished. This is the same thing what we dislike in
data providers where be_req_terminate was called on many places and it
made the code difficult and error prone.
E.g. this crash: e8474ac0be7e81c0ca54eb09e2fef42595602945
I'd like to do something similar as I did in data providers, but maybe
not in such extend.
2) Error replies -- there is just too much code around when you want to
reply with error.
I've already done something when working on sssctl but it needs to be
unified across the code. It is not much work.
E.g. a06e23c0bcf0c8669a29b801876aca8aac422931
3) Making it a separate static library as part of this effort would be
great. It is a perfect candidate.
> and Failover refactoring.
Yes, I was expecting you add this :) but I also agree this is probably
not in the scope..but then the sdap_id_ops refactoring would be even
more useful.