Review Request 130188: ktp-common-internals improvements

[David Edmundson]

> On July 28, 2017, 8:15 p.m., David Edmundson wrote:
> > KTp/Models/accounts-list-model.cpp, line 211
> > <https://git.reviewboard.kde.org/r/130188/diff/1/?file=498227#file498227line211>
> >
> >     why do we have to block?
> 
> James Smith wrote:
>     The call is async so the error value can be non-valid.
> 
> David Edmundson wrote:
>     Yes I can read what the code is doing, but why do we need to wait for that?
>     
>     We should be able to return true here, the model (should) get a dataChanged when it updates or not. Handle it outside this method.
>     
>     We're not merging any blocking calls in KTp.
> 
> James Smith wrote:
>     waitForFinished() blocks only the calling thread until the call's reply is returned and processed. This is the proper way to obtain valid return values from async calls, there is no way to guarantee a timely reply to the caller due to possible RPC mechanism contention and the non-blocking nature of async calls.
>     
>     Non-async calls block the RPC mechanism and as a consequence the caller, providing timely replies for the caller but potentially DoS'ing the called interface.
>     
>     Providing a valid return value is useful if the consumer needs to set the presence manually, for example the kded module is not available.

Read my second sentence.

Also the entirity of TelepathyQt is async DBus calls, if we can call requestPresence on a connection manager in a proper correct way, we can make a similar call to KDED in a correct way.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/130188/#review103499
-----------------------------------------------------------


On July 28, 2017, 6:08 p.m., James Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/130188/
> -----------------------------------------------------------
> 
> (Updated July 28, 2017, 6:08 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: ktp-common-internals
> 
> 
> Description
> -------
> 
> Integrate Status Handler. API additions and cleanup. Bugfixes and documentation.
> 
> 
> Diffs
> -----
> 
>   KTp/Declarative/qml-plugins.cpp 27ed202be547c088feaba92664b64617e3e541fc 
>   KTp/Models/accounts-list-model.h 09b1c28d1519dcfad24540008eab4ff0fadfd425 
>   KTp/Models/accounts-list-model.cpp 216102658db0d9183ccfe851807db3117e2df1f9 
>   KTp/Models/presence-model.h 7d35c9f529e51d1300926bc90a2674234ab87cfe 
>   KTp/Models/presence-model.cpp 9d32174df8b1052f7f01a7e31c8f5cbf44a445ee 
>   KTp/global-presence.h 46e6cc358b0f3cdaff35d3a7be25949646b400d3 
>   KTp/global-presence.cpp 0b751b145a227d59de564baa21bed1df0b23b125 
>   KTp/presence.h 901bd9898ae066e469cc6346d8f7433fef4108b2 
>   KTp/presence.cpp 5ac92b1aba7d3b8fb86840b5e86c50b778b5c8dc 
>   KTp/types.h 8d3ca4fc14a37b61abecdf78594f334f7b6aa797 
> 
> Diff: https://git.reviewboard.kde.org/r/130188/diff/
> 
> 
> Testing
> -------
> 
> Compile, run.
> 
> 
> Thanks,
> 
> James Smith
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20170817/fb501087/attachment.html>