> On March 18, 2015, 7:27 p.m., Floris-Andrei Stoica-Marcu wrote: > > That one delayed reply was there for a long time, I thought about removing > > it, but in the past did not see any change in doing so. It is also worth > > noting that considering the way the transactions are named and such (on the > > dbus), because they each are given an unique name. That tells me that, > > maybe there were plans to be able to install multiple packages at once. In > > that context the delayed reply kinda fits.
Yeah, I think in the long run we'd want to go somewhere async. If I am not mistaken you can actually already queue multiple transactions, by design they can't run at the same time though (what with apt locking). So it is probably only parts of the worker-side transaction that has flaky API design. Most likely functions probably shouldn't have return values but if they want to yield something into the client they would have to do it through an explicit signal, thus not running the risk of timing out the sync dbus function call. Which is as async as it gets. That being said I think run actually will need to be renamed to 'start' or perhaps better yet 'queue' to indicate that the function in of itself doesn't do anything other than obtain authorization and then queue the transaction for the worker to process at a *later* time. Something for QApt4 I guess. - Harald ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123026/#review77699 ----------------------------------------------------------- On March 18, 2015, 2:28 p.m., Harald Sitter wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/123026/ > ----------------------------------------------------------- > > (Updated March 18, 2015, 2:28 p.m.) > > > Review request for Kubuntu, LibQApt, Aleix Pol Gonzalez, and Floris-Andrei > Stoica-Marcu. > > > Repository: libqapt > > > Description > ------- > > setDelayedReply means we would have to issue a reply ourselves which from > what I can see never happens. So after a certain timeout (30 or so seconds) > dbus will automatically issue a NoReply error to the client. While this is > a truthy error in that there really was no reply in the time frame its > meaning client side is unclear as NoReply can then mean actually no reply > or everything was fine but no reply was issued. Latter appears to be the > success case because we never issue a reply, as already mentioned. > This in turn makes it impossible to tell client side when run simply took > too long or was successful. > > To improve the situation simply do not mark the reply delayed. This makes > qtdbus send a default all-good reply to the client unless we issue an auth > error first. > > Ideally the entire timeout scenario should be taken out of the picture by > employing an entirely async API design (i.e. run always replies immediately > but asyncronously switches state to either an error or running). The only > other option to prevent the noreply from being issued while waiting for > auth is to have the client timeout increased to indefinite which would seem > misguided as most of the operations have actual potential to result in > no-reply if something deadlocks or takes indeed way too long. > > > Diffs > ----- > > src/worker/transaction.cpp 4333b26b211774c560ff046ab51b638b83282f1c > > Diff: https://git.reviewboard.kde.org/r/123026/diff/ > > > Testing > ------- > > make && make install && make test > > # Runtime Test > > set apt download limit to 8kb/s http://askubuntu.com/a/50405 > > ## without the fix > > - start muon-updater > - do an update > - provide authorization > - after ~30 seconds the no-auth error comes up as per review #119793 > > ## with the fix > > make sure qaptworker3 is not running > > ### failure condition > > - start muon-updater > - do an update > - do not provide authorization > - after ~30 seconds the no-auth error comes up as per review #119793 > > ### success condition > > - start muon-updater > - do an update > - provide authorization > - update finishes without a no-auth error > > > Thanks, > > Harald Sitter > >
-- kubuntu-devel mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/kubuntu-devel
