[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