Bug 20905

Summary: Account: [impl needed] UpdateParameters + Reconnect
Product: Telepathy Reporter: Simon McVittie <smcv>
Component: tp-specAssignee: Simon McVittie <smcv>
Status: RESOLVED FIXED QA Contact: Telepathy bugs list <telepathy-bugs>
Severity: normal    
Priority: medium Keywords: patch
Version: unspecified   
Hardware: Other   
OS: All   
URL: http://git.collabora.co.uk/?p=user/smcv/telepathy-spec-smcv.git;a=shortlog;h=refs/heads/update-parameters
i915 platform: i915 features:
Bug Depends on: 21154    
Bug Blocks:    

Description Simon McVittie 2009-03-27 07:06:33 UTC
Mission Control 5 disconnects and reconnects the account whenever properties that are not a DBusProperty are changed, in violation of telepathy-spec. This is in order to achieve an "instant apply" model. However, I believe that MC is the wrong place for this policy enforcement.

Possible solutions for MC 5's use case:

* Change the spec to document MC 5's behaviour, overruling my objection

* (API break) make UpdateParameters return a boolean, "reconnect required", and let the client do the reconnect when it wants to - consider adding a Reconnect method too. libmcclient's method to update parameters could be patched to call Reconnect whenever necessary, keeping its C API unchanged

* Add a new parameter-updating method paralleling UpdateParameters that has one of the above behaviours

Comment 1 Simon McVittie 2009-04-09 09:27:27 UTC
Unfortunately, there is no method to do this in libmcclient, so existing code could be depending on the current instant-apply semantics. I can easily add one and port any existing clients.
Comment 2 Simon McVittie 2009-04-10 09:26:11 UTC
Proposed solution is to add a Reconnect method and break API on UpdateParameters  by returning a boolean to indicate whether Reconnect needs calling.


This requires easy patches to any existing clients.

Before merging, this should be implemented on an MC branch as a proof of concept.
Comment 3 Simon McVittie 2009-04-13 10:39:48 UTC
spec cabal says yes
Comment 4 Simon McVittie 2009-04-13 10:40:58 UTC
grammar cabal says no, re-introduce the stray semicolon
Comment 5 Simon McVittie 2009-04-13 11:19:17 UTC
Spec meeting approves in principle, I will update the branch to fix the grammar and get it insta-reviewed.
Comment 6 Simon McVittie 2009-04-13 11:19:33 UTC
grr bugzilla
Comment 7 Simon McVittie 2009-04-14 08:12:35 UTC
Account creation and UpdateParameters are now on a single branch:


We shouldn't merge these until they both have implementations in an MC branch.
Comment 8 Simon McVittie 2009-04-23 02:58:23 UTC
Here are some thoughts about the UI for this that we'll probably have on the GNOME desktop, since this motivates the API. The current plan, I think, is to save the new settings immediately, but display a banner with a reconnect button:

    /!\ Some of these changes will not take effect until you reconnect
        this account.    [ Reconnect now ]

The D-Bus API proposed above can support this.

Another possibility would be to display some sort of symbol or highlight visually matching the banner, on each input widget whose setting hasn't yet taken effect:

        Account [ smcv@example.com ]
        Password [ ***** ]
        [x] Override automatic server/port selection
    /!\ Server [ jabber.vpn.example.com ]
    /!\ Port [ 4321 ]
    /!\ The indicated changes will not take effect until you reconnect
        this account.    [ Reconnect now ]

This would require that UpdateParameters() returned a list of parameters (D-Bus: 'as') whose update will be delayed until the next reconnection. I think if we're breaking API anyway, we should consider breaking it in a maximally helpful way :-)
Comment 9 Simon McVittie 2009-04-28 08:40:08 UTC
Approved in principle by sjoerd/cassidy/wjt, spec wording to follow.
Comment 10 Simon McVittie 2009-06-04 07:07:54 UTC
Fixed in 0.17.24

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.