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).