11:02 a.m.
Hi Patrick,
When looking into why the Session has a GetConfigs dbus method, I
noticed that there are several dbus methods which are not documented
in syncevo-session-full.xml. This includes Attach which is actually
mention but not properly documented in that file.
Is this just a case of the docs being out of sync or are some of these
methods not intended to be in the session interface?
Cheers,
Chris
12:26 p.m.
On Mo, 2011-12-19 at 11:02 +0100, Chris Kühl wrote:
When looking into why the Session has a GetConfigs dbus method, I
noticed that there are several dbus methods which are not documented
in syncevo-session-full.xml. This includes Attach which is actually
mention but not properly documented in that file.
Good catch.
Is this just a case of the docs being out of sync or are some of
these
methods not intended to be in the session interface?
The docs are incomplete. In the patch which introduces the
implementation, I added some comments to the documentation, but I missed
completely that the actual documentation for Attach() and Detach() were
missing.
What is missing is something like:
<method name="Attach">
<doc:doc>
<doc:description>
Prevents destruction of the session until the client
detaches or quits. Implemented with a counter, so each
Attach() must be matched by a Detach() to free the session.
Meant to be used by clients which want to follow the
progress of a session started by the daemon or some other
client. StartSession() automatically includes one Attach()
call, so typically the client which created a session never
calls Attach() and Detach() only once.
</doc:description>
</method>
<method name="Detach">
<doc:doc>
<doc:description>
Each Attach() or StartSession() must be matched by one
Detach() call, otherwise the session will not be
destructed. A client that quits without these calls will be
detached automatically.
</doc:description>
</doc:doc>
</method>
For the record, here's the commit:
commit 320e23e31d163e858ab4e03488b288350f336004
Author: Patrick Ohly <patrick.ohly(a)intel.com>
Date: Thu Sep 30 10:32:13 2010 +0200
D-Bus API: added missing Session.Attach() (BMC #7761)
It was always part of the design that multiple clients can use any session.
Somehow we forgot to implement Session.Attach(), the call that allows a client
which hasn't started a session to register it's interest in the session. In
particular, that session won't go away unless the client exits or detaches.
This patch adds that missing call, a corresponding capability
("SessionAttach") and a unit test. The unit test covers recursive
attach/detach, it does not actually verify that the session remains
around.
diff --git a/src/dbus/interfaces/syncevo-server-full.xml
b/src/dbus/interfaces/syncevo-server-full.xml
index 861357b..82ba84d 100644
--- a/src/dbus/interfaces/syncevo-server-full.xml
+++ b/src/dbus/interfaces/syncevo-server-full.xml
@@ -65,6 +65,12 @@
</doc:definition>
</doc:item>
+ <doc:item><doc:term>SessionAttach</doc:term>
+ <doc:definition>Session.Attach()
+ implemented
+ </doc:definition>
+ </doc:item>
+
<doc:item><doc:term>Notifications</doc:term>
<doc:definition>Server.DisableNotifications()
and Server.EnableNotifications() implemented
diff --git a/src/dbus/interfaces/syncevo-session-full.xml
b/src/dbus/interfaces/syncevo-session-full.xml
index d81862b..fb77ecb 100644
--- a/src/dbus/interfaces/syncevo-session-full.xml
+++ b/src/dbus/interfaces/syncevo-session-full.xml
@@ -2,8 +2,18 @@
<node name="/"
xmlns:doc="http://www.freedesktop.org/dbus/1.0/doc.dtd">
<interface name="org.syncevolution.Session">
<doc:doc><doc:description>
- <doc:para>A Session object is used to do do syncs and to modify the server
configurations. Clients can create
- <doc:para>Commands (other than Detach()) cannot be used before the status
changes to "idle" (see GetStatus() a
+ <doc:para>
+ A Session object is used to do do syncs and to modify
+ the server configurations. Clients can create a Session with
+ Server.StartSession(), attach to a session started by some other
+ client with Session.Attach(), and detach from it with
+ Session.Detach().
+ </doc:para>
+ <doc:para>
+ Commands (other than Attach() and Detach()) cannot be used
+ before the status changes to "idle" (see GetStatus() and
+ StatusChanged).
+ </doc:para>
</doc:description></doc:doc>
<method name="GetFlags">
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
3:21 p.m.
On Mon, Dec 19, 2011 at 12:26 PM, Patrick Ohly <patrick.ohly(a)intel.com> wrote:
On Mo, 2011-12-19 at 11:02 +0100, Chris Kühl wrote:
> When looking into why the Session has a GetConfigs dbus method, I
> noticed that there are several dbus methods which are not documented
> in syncevo-session-full.xml. This includes Attach which is actually
> mention but not properly documented in that file.
Good catch.
So what is the reason for GetConfigs in the DBus Session API? It seems
to be identical to Server.GetConfigs.
> Is this just a case of the docs being out of sync or are some of these
> methods not intended to be in the session interface?
The docs are incomplete. In the patch which introduces the
implementation, I added some comments to the documentation, but I missed
completely that the actual documentation for Attach() and Detach() were
missing.
What is missing is something like:
<method name="Attach">
<doc:doc>
<doc:description>
Prevents destruction of the session until the client
detaches or quits. Implemented with a counter, so each
Attach() must be matched by a Detach() to free the session.
Meant to be used by clients which want to follow the
progress of a session started by the daemon or some other
client. StartSession() automatically includes one Attach()
call, so typically the client which created a session never
calls Attach() and Detach() only once.
</doc:description>
</method>
<method name="Detach">
<doc:doc>
<doc:description>
Each Attach() or StartSession() must be matched by one
Detach() call, otherwise the session will not be
destructed. A client that quits without these calls will be
detached automatically.
</doc:description>
</doc:doc>
</method>
For the record, here's the commit:
Ok, thanks.
Cheers,
Chris
3:24 p.m.
On Mo, 2011-12-19 at 15:21 +0100, Chris Kühl wrote:
On Mon, Dec 19, 2011 at 12:26 PM, Patrick Ohly
<patrick.ohly(a)intel.com> wrote:
> On Mo, 2011-12-19 at 11:02 +0100, Chris Kühl wrote:
>> When looking into why the Session has a GetConfigs dbus method, I
>> noticed that there are several dbus methods which are not documented
>> in syncevo-session-full.xml. This includes Attach which is actually
>> mention but not properly documented in that file.
>
> Good catch.
So what is the reason for GetConfigs in the DBus Session API? It seems
to be identical to Server.GetConfigs.
It's just there for the sake of completeness.
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
3:49 p.m.
On Mon, Dec 19, 2011 at 3:24 PM, Patrick Ohly <patrick.ohly(a)intel.com> wrote:
On Mo, 2011-12-19 at 15:21 +0100, Chris Kühl wrote:
> On Mon, Dec 19, 2011 at 12:26 PM, Patrick Ohly <patrick.ohly(a)intel.com> wrote:
> > On Mo, 2011-12-19 at 11:02 +0100, Chris Kühl wrote:
> >> When looking into why the Session has a GetConfigs dbus method, I
> >> noticed that there are several dbus methods which are not documented
> >> in syncevo-session-full.xml. This includes Attach which is actually
> >> mention but not properly documented in that file.
> >
> > Good catch.
>
> So what is the reason for GetConfigs in the DBus Session API? It seems
> to be identical to Server.GetConfigs.
It's just there for the sake of completeness.
This DBus method invokes the getConfigs method in ReadOperations. In
turn, this calls Server::getDeviceList which requires BluezManager.
I'd rather not have to create a new BluezManager in the Session sync
process or cause the Session.GetConfigs to trigger a call to the
Server's GetConfigs method (or it's peer-to-peer dbus equivalent). If
it's, in fact, not really needed, is it ok if I drop this from the
Session dbus api ...at least for now?
Cheers,
Chris
4:08 p.m.
On Mo, 2011-12-19 at 15:49 +0100, Chris Kühl wrote:
On Mon, Dec 19, 2011 at 3:24 PM, Patrick Ohly
<patrick.ohly(a)intel.com> wrote:
> On Mo, 2011-12-19 at 15:21 +0100, Chris Kühl wrote:
>> On Mon, Dec 19, 2011 at 12:26 PM, Patrick Ohly <patrick.ohly(a)intel.com>
wrote:
>> > On Mo, 2011-12-19 at 11:02 +0100, Chris Kühl wrote:
>> >> When looking into why the Session has a GetConfigs dbus method, I
>> >> noticed that there are several dbus methods which are not documented
>> >> in syncevo-session-full.xml. This includes Attach which is actually
>> >> mention but not properly documented in that file.
>> >
>> > Good catch.
>>
>> So what is the reason for GetConfigs in the DBus Session API? It seems
>> to be identical to Server.GetConfigs.
>
> It's just there for the sake of completeness.
>
This DBus method invokes the getConfigs method in ReadOperations. In
turn, this calls Server::getDeviceList which requires BluezManager.
I'd rather not have to create a new BluezManager in the Session sync
process or cause the Session.GetConfigs to trigger a call to the
Server's GetConfigs method (or it's peer-to-peer dbus equivalent). If
it's, in fact, not really needed, is it ok if I drop this from the
Session dbus api ...at least for now?
Oh, now I understand the original question. GetConfigs() is available
for a session because it is always provided by ReadOperations, right? It
was never documented as a session method. So in that sense it can be
considered an implementation detail which can be removed.
But why does the method have to be executed in the session process?
Isn't the caller talking to the main process who needs to dispatch calls
through to the background process? What I am aiming at is that the main
process could execute this particular method as if it had been invoked
on the server interface.
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
5:19 p.m.
On Mon, Dec 19, 2011 at 4:08 PM, Patrick Ohly <patrick.ohly(a)intel.com> wrote:
On Mo, 2011-12-19 at 15:49 +0100, Chris Kühl wrote:
> On Mon, Dec 19, 2011 at 3:24 PM, Patrick Ohly <patrick.ohly(a)intel.com> wrote:
> > On Mo, 2011-12-19 at 15:21 +0100, Chris Kühl wrote:
> >> On Mon, Dec 19, 2011 at 12:26 PM, Patrick Ohly
<patrick.ohly(a)intel.com> wrote:
> >> > On Mo, 2011-12-19 at 11:02 +0100, Chris Kühl wrote:
> >> >> When looking into why the Session has a GetConfigs dbus method, I
> >> >> noticed that there are several dbus methods which are not
documented
> >> >> in syncevo-session-full.xml. This includes Attach which is
actually
> >> >> mention but not properly documented in that file.
> >> >
> >> > Good catch.
> >>
> >> So what is the reason for GetConfigs in the DBus Session API? It seems
> >> to be identical to Server.GetConfigs.
> >
> > It's just there for the sake of completeness.
> >
>
> This DBus method invokes the getConfigs method in ReadOperations. In
> turn, this calls Server::getDeviceList which requires BluezManager.
> I'd rather not have to create a new BluezManager in the Session sync
> process or cause the Session.GetConfigs to trigger a call to the
> Server's GetConfigs method (or it's peer-to-peer dbus equivalent). If
> it's, in fact, not really needed, is it ok if I drop this from the
> Session dbus api ...at least for now?
Oh, now I understand the original question. GetConfigs() is available
for a session because it is always provided by ReadOperations, right? It
was never documented as a session method. So in that sense it can be
considered an implementation detail which can be removed.
But why does the method have to be executed in the session process?
Isn't the caller talking to the main process who needs to dispatch calls
through to the background process? What I am aiming at is that the main
process could execute this particular method as if it had been invoked
on the server interface.
As mentioned in the first of 3 steps outlining my plans for this work,
I'm attempting to break the session apart from the server; decoupling
it completely. This is a pain and has caused me to consider only
running the sync in the separate process several times. However, I
feel it would be much cleaner (but definitely not easier) if the
session, along with its dbus api, lives in its own process. Ideally,
the dbus api shouldn't change for consumers of the api, but if
GetConfigs is not really needed or used then it'd make life easier to
nix it.
Cheers,
Chris
3806
days inactive
3806
days old
syncevolution@synevolution.org
6 comments
2 participants
participants (2)
-
Chris Kühl -
Patrick Ohly