Merge lp:~nikwen/account-polld/donechan-fix into lp:~ubuntu-push-hackers/account-polld/trunk
Proposed by
Niklas Wenzel
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | John Lenton | ||||
Approved revision: | 104 | ||||
Merged at revision: | 113 | ||||
Proposed branch: | lp:~nikwen/account-polld/donechan-fix | ||||
Merge into: | lp:~ubuntu-push-hackers/account-polld/trunk | ||||
Diff against target: |
60 lines (+13/-2) 1 file modified
cmd/account-polld/account_manager.go (+13/-2) |
||||
To merge this branch: | bzr merge lp:~nikwen/account-polld/donechan-fix | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
John Lenton (community) | Approve | ||
Review via email: mp+238898@code.launchpad.net |
Description of the change
The poll() function in "cmd/account-
Excerpt from the logs:
2014/10/19 17:23:15 Starting poll for account 4
2014/10/19 17:23:15 Polling account 4
2014/10/19 17:23:15 Account 4 failed to authenticate: GDBus.Error:
2014/10/19 17:23:45 Poll for account 4 has timed out out after 30s
2014/10/19 17:23:45 Ending poll for account 4
To post a comment you must log in.
Finally have time to review this! Sorry for the delay.
While the bug is real, I have nits with your fix:
the doneChan is a channel for errors, but you're sending nil (no error) for at least two cases that are clearly errors. This needs fixing; I suggest returning instead ad-hoc errors and logging them in a single place (so e.g. returning fmt.Errorf("failed to authenticate: %v", a.authData.Error) instead of log-and-return-nil; then the "has failed:", err in the error path should suffice).
This will have the side effect of bumping penaltyCount for auth errors and click-not- installed, which is not necessarily a bad thing.