On Fri, Mar 28, 2014 at 12:45:01PM +0100, Pavel Březina wrote:
On 03/23/2014 10:08 PM, Jakub Hrozek wrote:
>Hi,
>
>I've been working on re-adding a public DBus API to SSSD lately, based
>on Stephen's InfoPipe code. While I'm not finished yet, the interface is
>also important to Pavel's OpenLMI thesis, so I'll be sending the patches
>for review as the individual subtasks are finished and rebased on top of
>Stef's recent patches.
>
>Attached are two patches that I think are ready to be reviewed and
>merged with some minor exceptions or questions. The review would be a
>good opportunity to solve those.
>
>[PATCH 1/2] IFP: Re-add the InfoPipe server
>This commit only adds the responder and the needed plumbing. No DBus
>related code is in yet.
>
>With this patch, I was wondering whether to build the code by default,
>or only when --enable-experimental-features is set until the whole
>feature is finished?
I don't think this is necessary.
>
> Also, with the current code, all responders spawn a client socket now,
>but this responder listens on the system bus instead. I created
>https://fedorahosted.org/sssd/ticket/2290 for this purpose -- Pavel,
>feel free to take it.
Will do.
>
>[PATCH 2/2] IFP: Connect to the system bus
>Adds the possibility for the InfoPipe responder to connect to the system
>bus. At the moment, only a dummy method "Ping" is provided. The method
>only accepts a single string parameter that has to be 'ping'.
>
>To test, you can call the Ping method like this:
>dbus-send --print-reply --system --dest=org.freedesktop.sssd.infopipe
>/org/freedesktop/sssd/infopipe org.freedesktop.sssd.infopipe.Ping
>'string:Ping'
>
>Or test that the code can catch wrong options:
> dbus-send --print-reply --system --dest=org.freedesktop.sssd.infopipe
>/org/freedesktop/sssd/infopipe org.freedesktop.sssd.infopipe.Ping
>'string:sdf'
>
>I'll be sending next round of patches later this week. Hopefully this
>would unblock Pavel's work.
Hi,
Patch 1:
>+%package dbus
>+Summary: The DBus responder if the SSSD
^ typo
>+Group: Applications/System
>+License: GPLv3+
>+Requires: sssd-common = %{version}-%{release}
Also I think we should use D-Bus in package description (and in
general) -- that's the correct spelling.
>+ <refnamediv id='name'>
>+ <refname>sssd-ifp</refname>
>+ <refpurpose>SSSD InfoPipe provider</refpurpose>
^ responder
>+ </refnamediv>
Ack to the code.
Patch 2:
Ack.
Nice work.
Thanks for the review. Two new patches are attached and one is a
completely new one that fixes a typo in an old code that was still
around (which is why I'm sending it separately).