URL: https://github.com/SSSD/sssd/pull/94
Title: #94: Enable {socket,dbus}-activation for responders
fidencio commented:
"""
On Thu, Dec 1, 2016 at 1:03 PM, Pavel Březina <notificati...@github.com>
wrote:
> Hi,
>
> if (ret != EOK) {
> #ifdef HAVE_SYSTEMD
> if (ret != ENOENT)
> #endif
> {
> DEBUG(SSSDBG_FATAL_FAILURE, "No services configured!\n");
> return EINVAL;
> }
> }
>
> can you separate above to:
>
> #ifdef HAVE_SYSTEMDif (ret != EOK && ret != ENOENT) {
> DEBUG(SSSDBG_FATAL_FAILURE, "No services configured!\n");
> return EINVAL;
> }
> #else
> ...
> #endif
>
>
Okay,
> We should also amend services option man page to describe what happens if
> a service is not listed there, that they are automatically activated when
> needed
>
Okay.
> .
>
> Now I have few more comments to the timeouts.
>
> 1. I believe you can remove "RESPONDER: Shutdown
> {dbus,socket}-activated responders in case they're idle" in favor of
> "Change the timeout logic".
>
>
The "Change timeout logic" commit is a leftover that must have been
squashed to "RESPONDER: Shutdown {dbus,socket}-activated responders in case
they're idle". Sorry for messing it up.
Squashing it there is okay for you?
>
> 1. Please use the same logic in sbus code. I think you just want to
> pass a pointer to last_request_time into sbus (to remove the need for
> function pointers) and let the sbus code update it when appropriate even
> for our private communication (in other responders). Because if a
> communication is happening between responder and provider, the responder is
> still not idle (it may be awaiting reply from data provider so it can be
> send to the client).
>
>
Okay, I can pass just the pointer to the last_request_time var into sbus.
Please, here I need a bit more pointers in order to be sure I understand
your suggestion.
Please, correct me if I'm wrong, it's updating the last_request_time even
for our private communication, no?
Are you talking specifically about IFP provider or the others? Because the
others have their last_request_time() updated everytime something goes
through their sockets and, as far as I understand, it should be enough,
right? If not, why not?
>
> 1. Resetting the timeout when sbus signal is received is not enough.
> You want to reset it everytime any communication on the bus is happening. I
> actually think that signals are the only communication that we can skip for
> two reasons (One - we don't use any signals except NameOwnerChanged, Second
> - those are asynchronous events that do not require any reply). I think you
> want to reset the time in sbus_message_handler when you got a valid
> handler (interface and method combination. The you don't need the iface
> validator.
>
>
Hmmm. I see your point and it makes sense. But I have one question ... We
do receive signals from org.freedesktop.sssd.infopipe.* and they should be
treated, right? So we can't ignore it completely.
In case those signals end up in sbus_message_handler() as well, then I
agree with you. Is that the case?
>
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/SSSD/sssd/pull/94#issuecomment-264156064>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AAG4ev7GREjEnsOvsuU94yxTOH6T1zj5ks5rDreogaJpZM4K8AJs>
> .
>
Thanks a lot for the review, Pavel.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/94#issuecomment-264162692
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
