[Merge] lp:~renatofilho/account-polld/gcalendar-plugin into lp:account-polld
Jonas G. Drange
jonas.drange at canonical.com
Tue Apr 12 12:48:54 UTC 2016
LGTM, except the change to main.go, which I've added an inline comment about.
Thanks, Renato!
Diff comments:
> === modified file 'cmd/account-polld/main.go'
> --- cmd/account-polld/main.go 2015-12-19 19:54:01 +0000
> +++ cmd/account-polld/main.go 2016-04-11 17:19:05 +0000
> @@ -82,7 +85,9 @@
>
> pollBus := pollbus.New(bus)
> go postOffice(bus, postWatch)
> - go monitorAccounts(postWatch, pollBus)
> + for _,accountType := range SERVICETYPES {
I think we should keep only one monitorAccounts call since that method is quite complex (blocking for { select { … }} loop). Maybe create two watchers, one for webapps and one for calendar inside monitorAccounts? I.e.
webAppsAccWatcher := accounts.NewWatcher(webapps)
calendarAccWatcher := accounts.NewWatcher(calendar)
for {
select {
case data := <-webAppsAccWatcher.C:
case data := <-calendarAccWatcher.C:
Or something?
Because these two parallel monitorAccounts threads might account for the current problems with this code.
> + go monitorAccounts(postWatch, pollBus, accountType)
> + }
>
> if err := pollBus.Init(); err != nil {
> log.Fatal("Issue while setting up the poll bus:", err)
--
https://code.launchpad.net/~renatofilho/account-polld/gcalendar-plugin/+merge/291393
Your team Ubuntu Push Hackers is requested to review the proposed merge of lp:~renatofilho/account-polld/gcalendar-plugin into lp:account-polld.
More information about the Ubuntu-reviews
mailing list