edit: stylistic refactors

cmd/pop/edit.go

@@ -49,7 +49,7 @@ func editCmd() *cli.Command {
 			return err
 		}
 
-		srv, err := edit.HTTPServer(ctx, *p, edit.Opts{Client: c})
+		srv, err := edit.HTTPHandler(ctx, *p, edit.Opts{Client: c})
 		if err != nil {
 			return err
 		}

pkg/api/fuge/fuge.go

@@ -1,3 +1,6 @@
+// package fuge wraps the centrifuge API to make using it more natural in Go:
+// - func values instead of full types
+// - forced synchronization when connecting and subscribing (and thus error return values)
 package fuge
 
 import (

pkg/api/grafana/grafana.go

@@ -10,23 +10,28 @@ import (
 	grafana "github.com/grafana/grafana-api-golang-client"
 )
 
+// Client provides access to the several API layers of Grafana until a proper
+// client package is in place.
 type Client struct {
 	gapi *grafana.Client
 	auth Auth
 	url  *url.URL
 }
 
+// Auth holds authentication information for the Grafana API
 type Auth struct {
 	Token string
 	Basic *url.Userinfo
 	OrgID int64
 }
 
+// Info holds information about the remote Grafana instance
 type Info struct {
 	Commit  string
 	Version semver.Version
 }
 
+// New returns a grafana.Client for the Grafana instance at baseURL
 func New(baseURL string, auth Auth) (*Client, error) {
 	base, err := url.Parse(baseURL)
 	if err != nil {
@@ -54,10 +59,13 @@ func New(baseURL string, auth Auth) (*Client, error) {
 	return c, nil
 }
 
+// API returns an actual grafana/grafana-api-golang-client, which implements
+// most operations
 func (c *Client) API() *grafana.Client {
 	return c.gapi
 }
 
+// Info returns metadata about the remote Grafana instance
 func (c *Client) Info() (*Info, error) {
 	u := *c.url
 	u.Path = path.Join(u.Path, "api", "health")

pkg/api/grafana/watch.go

@@ -11,15 +11,18 @@ import (
 	"github.com/pollypkg/polly/pkg/api/fuge"
 )
 
+// Watcher listens for dashboard change events and dispatches registered actions
+// when they happen
 type Watcher struct {
 	c *fuge.Fuge
-	e chan error
 
 	gapi *grafana.Client
 	subs map[string]*fuge.Sub
 	auth Auth
 }
 
+// NewWatcher returns a new watcher already connected to Grafana's centrifuge
+// API. Use Add() to subscribe to the actual dashboard change events.
 func (c *Client) NewWatcher() (*Watcher, error) {
 	i, err := c.Info()
 	if err != nil {
@@ -45,7 +48,6 @@ func (c *Client) NewWatcher() (*Watcher, error) {
 
 	w := Watcher{
 		c: f,
-		e: make(chan error),
 
 		gapi: c.gapi,
 		subs: make(map[string]*fuge.Sub),
@@ -54,8 +56,12 @@ func (c *Client) NewWatcher() (*Watcher, error) {
 	return &w, nil
 }
 
-type ChangeHandler func(map[string]interface{}) error
+// ChangeHandler is invoked when a dashboard change event occurs. It is passed
+// either the dashboard model, or an error that occured.
+type ChangeHandler func(map[string]interface{}, error)
 
+// Add susbcribes to change events for the dashboard of given UID and invokes
+// handler when those occur.
 func (w *Watcher) Add(uid string, handler ChangeHandler) error {
 	channel := fmt.Sprintf("%d/grafana/dashboard/uid/%s", w.auth.OrgID, uid)
 	onPub := func(s *centrifuge.Subscription, e centrifuge.PublishEvent) {
@@ -66,20 +72,17 @@ func (w *Watcher) Add(uid string, handler ChangeHandler) error {
 		}
 
 		if err := json.Unmarshal(e.Data, &event); err != nil {
-			w.e <- err
+			handler(nil, err)
 			return
 		}
 
 		d, err := w.gapi.DashboardByUID(uid)
 		if err != nil {
-			w.e <- err
+			handler(nil, err)
 			return
 		}
 
-		if err := handler(d.Model); err != nil {
-			w.e <- err
-			return
-		}
+		handler(d.Model, nil)
 	}
 
 	s, err := w.c.Sub(channel, onPub)
@@ -91,6 +94,7 @@ func (w *Watcher) Add(uid string, handler ChangeHandler) error {
 	return nil
 }
 
+// Del removes the subscription for the dashboard of given UID if there is one.
 func (w *Watcher) Del(uid string) {
 	s, ok := w.subs[uid]
 	if !ok {
@@ -101,10 +105,6 @@ func (w *Watcher) Del(uid string) {
 	delete(w.subs, uid)
 }
 
-func (w *Watcher) Err() error {
-	return <-w.e
-}
-
 func (w *Watcher) Close() error {
 	var lastErr error
 	for _, s := range w.subs {

pkg/edit/edit.go

@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
+	"log"
 	"os"
 	"strings"
 
@@ -86,22 +87,25 @@ func (c Grafana) Add(name string) error {
 		return err
 	}
 
+	// TODO(sh0rez): this is hacky, better formalize (also the undoing below)
 	originalUID := i["uid"]
 	editUID := genEditUID(name)
 	i["uid"] = editUID
 	i["id"] = nil
 
 	_ = c.api.DeleteDashboardByUID(editUID)
-	_, err = c.api.NewDashboard(gapi.Dashboard{
-		Model: i,
-	})
-	if err != nil {
+	if _, err = c.api.NewDashboard(gapi.Dashboard{Model: i}); err != nil {
 		return fmt.Errorf("Failed to create temporary dashboard '%s' in Grafana: %w", editUID, err)
 	}
 
 	c.inEdit[name] = editUID
 
-	err = c.watch.Add(editUID, func(upd map[string]interface{}) error {
+	err = c.watch.Add(editUID, func(upd map[string]interface{}, err error) {
+		if err != nil {
+			log.Printf("Error: Failed receiving update event for '%s': %s", editUID, err)
+			return
+		}
+
 		upd["uid"] = originalUID
 		delete(upd, "id")
 		delete(upd, "version")
@@ -117,6 +121,8 @@ func (c Grafana) Add(name string) error {
 
 		data, err := json.MarshalIndent(model, "", "  ")
 		if err != nil {
+			// failure here suggests programming mistakes, as model must be
+			// serializable
 			panic(err)
 		}
 
@@ -126,21 +132,16 @@ func (c Grafana) Add(name string) error {
 
 		fmted, err := format.Source([]byte(pkged), format.Simplify())
 		if err != nil {
+			// must not fail, as JSON is valid CUE by definition
 			panic(err)
 		}
 
 		if err := ioutil.WriteFile(file, fmted, 0744); err != nil {
-			panic(err)
+			log.Printf("Error: Failed to write updated '%s' to disk: %s", file, err)
 		}
-
-		return nil
 	})
 
-	if err != nil {
-		return err
-	}
-
-	return nil
+	return err
 }
 
 // EditUID returns the uid of the current editing session of the dashboard
@@ -200,7 +201,7 @@ func cuePackage(file string) (string, error) {
 }
 
 // Trim removes unwanted fields from the dashboard model in-place.
-// TODO: merge with removing version, id, etc
+// TODO(sh0rez): merge with removing version, id, etc
 // TODO: use schema to trim defaults
 func Trim(i interface{}) {
 	switch i := i.(type) {

pkg/edit/file.go

@@ -78,6 +78,6 @@ func (e ErrMultipleFiles) Error() string {
 }
 
 func isSchema(f string) bool {
-	// TODO: this is a very naive and weak assumption. find a better one
+	// TODO(sh0rez): this is a very naive and weak assumption. find a better one
 	return strings.HasSuffix(filepath.ToSlash(f), "polly/schema/pollypkg.cue")
 }