10:26 < danni> smcv: is there an official way for an observer to find out about all existing channels? 10:27 < sjoerd> danni: nope 10:27 < sjoerd> (yes this is a weakness) 10:27 < danni> so is the solution "more API" or "ask AM for a list of Connections and then iterate them for channels you care about" 10:28 < sjoerd> long-term or short-term ? 10:28 < danni> both? 10:28 < sjoerd> first is long-term, second is short-term 10:29 < danni> ok 10:29 < sjoerd> danni: if you could file a bug against tp-spec, that would be great
Had a thought today. This could simply be a signal on the Observer interface that can Observer can emit, that way the CD can know the emitters path and have a look at the appropriate filter before calling ObserveChannels.
(In reply to comment #1) > Had a thought today. This could simply be a signal on the Observer interface > that can Observer can emit, that way the CD can know the emitters path and have > a look at the appropriate filter before calling ObserveChannels. I don't think it can be: how can the observer know that MissionControl is ready to receive its signals?
12:55 < smcv> I think this is actually a special case of the "please restart me" flag 12:56 < danni> smcv: you may not have been restarted of course, you may have been started for the first time 12:56 < smcv> PleaseRestartMe && activatable -> (re)start the observer whenever a channel it's interested in appears, or whenever it exits while such a channel exists 12:56 < danni> and want to know what's happening 12:56 < smcv> PleaseRestartMe && !activatable -> whenever the observer pops up, call ObserveChannels for each channel it would have been interested in 12:57 < smcv> in any case, someone (ptlo?) is working on the former case, which makes the latter case considerably simpler to implement even if it ends up not being the same API PleaseRestartMe is Bug #24376.
If we implement this by calling ObserveChannels() again, there ought to be something in Observer_Info ({'catch-up': True} perhaps?) to indicate that they're not really new channels.
Branch for implementing this: http://git.collabora.co.uk/?p=user/ptlo/tp-spec-senko/.git;a=shortlog;h=refs/heads/respawnable-observers
+ <p>If true, upon the startup of this observer, ObserveChannels will be + called for every already existing channel that this observer wants to + observe (ie. every channel that would cause ObserveChannels to be called + on this observer upon creation).</p> Insert a blank line between this paragraph and the next, please. + <p>If the observer is an activatable client, ObserveChannels won't be + emitted after the client is ready. Instead, it will be omitted immediately + when the observer's well-known name disappears from the bus, so the client + will be restarted immediately via DBus service activation.</p> Methods are "called", not "emitted". They're certainly not "omitted", which is something else entirely :-) Also, I think this wording is a bit confusing. How about this? <p>If the Observer is service-activatable, instead of watching passively for a new instance of it to start, the channel dispatcher will call its ObserveChannels method as soon as its well-known name disappears, resulting in it being restarted immediately via D-Bus service activation.</p> ... and I'd like some rationale, like this: <tp:rationale> <p>This means that if an activatable Observer crashes, it will be restarted as soon as possible; while there is an unavoidable possibility that it will miss some events during this process (particularly <tp:dbus-ref namespace="org.freedesktop.Channel.Type">Text</tp:dbus-ref> messages), this window of event loss is kept to a minimum.</p> <p>Non-activatable observers can't take advantage of this mechanism, but setting this property on a non-activatable observer does allow it to "catch up" on channels that are currently active at the time that it starts up.</p> </tp:rationale>
> <p>When the ObserveChannels method is called due to observer recovery, > the "Observer_Info" dictionary will contain one extra item with key > "ExistingChannel" and boolean value of True.</p> Our case convention for "extra misc info" dicts is usually like GObject properties (so, "existing-channel" in this case). I think this key should be called "recovering", actually. The Observer_Info parameter to ObserveChannels should document it, too. Pseudocode, in which /foo/ indicates a hyperlink: recovering : b If present and TRUE, this channel already existed before the observer started, and is being supplied to it due to the /Recover/ property (see that property for details). > <tp:rationale> > <p>The observers might hold state or do channel-specific initialisation > for the channels they observe. This is a way of signalling to them > that they were already observing this channel, so they don't have to > do extra round-trips to query for this information.</p> I think this rationale is wrong. If you receive a channel with 'recovering', then by definition you've just started up, so you have no state! Instead, the rationale for the "recovering" key should be something like: This allows observers to distinguish between new channels (the normal case), and existing channels that were given to the observer in order to catch up on previous events (perhaps after a previous instance of the same observer crashed). Do you have an implementation of this in MC? I'll clone this bug for the implementation; please attach any implementation for review to the clone, with the patch keyword. This spec change can potentially be merged without an implementation in MC, but can't be undrafted until we have a suitable implementation. To be honest, since this branch is so small, I'd be inclined to implement it in MC based on this draft, and not bother to merge it to the spec until we have a reasonable implementation on a branch, at which point it can go straight in as stable API. When this is ready for review again, please remove review- from the status whiteboard and re-add the patch keyword.
*** Bug 24376 has been marked as a duplicate of this bug. ***
12:33 < ptlo> 12:22:50> smcv, wrt fd.o comment #24768, I didn't document the recover flag in Observer_Info doc because that's in Client.Observer (which AIUI is not to be touched with unstable features), not Client.Observer.FUTURE; it didn't seem right to copy/paste the entire ObserveChannels/Observer_Info doc into .FUTURE just to be able to add the flag there That's fair enough, but please provide (either on this bug, or somewhere in the branch - e.g. text headed <strong>Text to be added to Observer_Info</strong> in the FUTURE XML, or something) the text that will be *added* to Observer_Info, so that undrafting this feature is just a matter of moving text around.
(In reply to comment #9) > 12:33 < ptlo> 12:22:50> smcv, wrt fd.o comment #24768, I didn't document the > recover flag in Observer_Info doc because that's in > Client.Observer (which AIUI is not to be touched with unstable > features), not Client.Observer.FUTURE; it didn't seem right to > copy/paste the entire ObserveChannels/Observer_Info doc into > .FUTURE just to be able to add the flag there > > That's fair enough, but please provide (either on this bug, or somewhere in the > branch - e.g. text headed <strong>Text to be added to Observer_Info</strong> in > the FUTURE XML, or something) the text that will be *added* to Observer_Info, > so that undrafting this feature is just a matter of moving text around. Changed according to review comments, squashed into a single commit: http://git.collabora.co.uk/?p=user/ptlo/tp-spec-senko/.git;a=commitdiff;h=9c0ab8aa46108d63980192e9cd955e8a9f99d2ff
(In reply to comment #10) > http://git.collabora.co.uk/?p=user/ptlo/tp-spec-senko/.git;a=commitdiff;h=9c0ab8aa46108d63980192e9cd955e8a9f99d2ff "MailNotification: UnreadMailCount >= UnreadMails array size"? I don't think that's the patch you meant :-) Assuming that <http://git.collabora.co.uk/?p=user/ptlo/tp-spec-senko/.git;a=shortlog;h=refs/heads/respawnable-observers> is what you meant me to review, I approve. However, since this is such a small spec change, it may be easier to not bother merging it as a draft - we could probably just insta-undraft it, if there's a good implementation already, so I'll review the MC implementation of this feature next.
The second try at an implementation over on Bug #27309 is good enough to reassure me that this API is implementable, so I think we can go directly to an undrafted version of this.
(In reply to comment #12) > The second try at an implementation over on Bug #27309 is good enough to > reassure me that this API is implementable, so I think we can go directly to an > undrafted version of this. Here's the undrafted version for instareview: http://git.collabora.co.uk/?p=user/ptlo/tp-spec-senko/.git;a=commitdiff;h=871718924be6e3bcaf4ffb7f0e7aafe75462b245
The undrafted version looks good to me, but please don't merge without spec-meeting approval. Thanks for getting this working!
Spec cabal approves, I'll merge it for the next spec release.
Stable API in 0.19.4.
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.