Fix linter (#30)

.golangci.yml

@@ -9,24 +9,65 @@ output:
 
 linters:
   enable:
+    - errcheck
+    - gofmt
+    - goimports
+    - gosec
     - govet
-    - whitespace
-
-  disable:
-    - godox
     - ineffassign
-    - errcheck
     - staticcheck
-    - gosimple
-    - stylecheck
-    - wsl
+    - typecheck
     - revive
+    - unused
+  disable:
+    - gochecknoinits
+    - gochecknoglobals
 
-linter-settings:
+linters-settings:
   govet:
     # report about shadowed variables
     check-shadowing: true
 
     # enable or disable analyzers by name
     # run `go tool vet help` to see all analyzers
-    enable-all: true
+    enable-all: true
+  golint:
+    min-confidence: 0.8
+  gocritic:
+    enabled-checks:
+      - appendCombine
+      - argOrder
+      - badCond
+      - dupBranchBody
+      - dupCase
+      - dupSubExpr
+      - elseif
+      - hugeParam
+      - initClause
+      - rangeValCopy
+      - sloppyLen
+      - typeSwitchVar
+      - underef
+      - unlambda
+      - unslice
+  gofmt:
+    simplify: true
+  goimports:
+    local-prefixes: github.com/myorg/mypackage
+  errcheck:
+    check-type-assertions: true
+    check-blank: true
+
+issues:
+  exclude-use-default: false
+  exclude-rules:
+    - linters:
+        - govet
+      text: "composite literal uses unkeyed fields"
+    - linters:
+        - golint
+      text: "should have comment or be unexported"
+    - linters:
+        - staticcheck
+      text: "SA5001: should check returned error before deferring"
+

integration-tests/batch_quickstart_test.go

@@ -52,7 +52,7 @@ func getPinotClientFromBroker() *pinot.Connection {
 	return pinotClient
 }
 
-func getCustomHttpClient() *http.Client {
+func getCustomHTTPClient() *http.Client {
 	httpClient := &http.Client{
 		Timeout: 15 * time.Second,
 		Transport: &http.Transport{
@@ -69,24 +69,24 @@ func getCustomHttpClient() *http.Client {
 	return httpClient
 }
 
-func getPinotClientFromZookeeperAndCustomHttpClient() *pinot.Connection {
-	pinotClient, err := pinot.NewFromZookeeperAndClient([]string{"localhost:" + zookeeperPort}, "", "QuickStartCluster", getCustomHttpClient())
+func getPinotClientFromZookeeperAndCustomHTTPClient() *pinot.Connection {
+	pinotClient, err := pinot.NewFromZookeeperAndClient([]string{"localhost:" + zookeeperPort}, "", "QuickStartCluster", getCustomHTTPClient())
 	if err != nil {
 		log.Fatalln(err)
 	}
 	return pinotClient
 }
 
-func getPinotClientFromControllerAndCustomHttpClient() *pinot.Connection {
-	pinotClient, err := pinot.NewFromControllerAndClient("localhost:"+controllerPort, getCustomHttpClient())
+func getPinotClientFromControllerAndCustomHTTPClient() *pinot.Connection {
+	pinotClient, err := pinot.NewFromControllerAndClient("localhost:"+controllerPort, getCustomHTTPClient())
 	if err != nil {
 		log.Fatalln(err)
 	}
 	return pinotClient
 }
 
-func getPinotClientFromBrokerAndCustomHttpClient() *pinot.Connection {
-	pinotClient, err := pinot.NewFromBrokerListAndClient([]string{"localhost:" + brokerPort}, getCustomHttpClient())
+func getPinotClientFromBrokerAndCustomHTTPClient() *pinot.Connection {
+	pinotClient, err := pinot.NewFromBrokerListAndClient([]string{"localhost:" + brokerPort}, getCustomHTTPClient())
 	if err != nil {
 		log.Fatalln(err)
 	}
@@ -101,9 +101,9 @@ func TestSendingQueriesToPinot(t *testing.T) {
 		getPinotClientFromZookeeper(),
 		getPinotClientFromController(),
 		getPinotClientFromBroker(),
-		getPinotClientFromZookeeperAndCustomHttpClient(),
-		getPinotClientFromControllerAndCustomHttpClient(),
-		getPinotClientFromBrokerAndCustomHttpClient(),
+		getPinotClientFromZookeeperAndCustomHTTPClient(),
+		getPinotClientFromControllerAndCustomHTTPClient(),
+		getPinotClientFromBrokerAndCustomHTTPClient(),
 	}
 
 	table := "baseballStats"

pinot/brokerSelector.go

@@ -1,3 +1,4 @@
+// Package pinot provides a client for Pinot, a real-time distributed OLAP datastore.
 package pinot
 
 type brokerSelector interface {

pinot/connection.go

@@ -30,10 +30,12 @@ func (c *Connection) ExecuteSQL(table string, query string) (*BrokerResponse, er
 	return brokerResp, err
 }
 
+// OpenTrace for the connection
 func (c *Connection) OpenTrace() {
 	c.trace = true
 }
 
+// CloseTrace for the connection
 func (c *Connection) CloseTrace() {
 	c.trace = false
 }

pinot/connectionFactory.go

@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"net/http"
 	"strings"
+
+	log "github.com/sirupsen/logrus"
 )
 
 const (
@@ -100,7 +102,10 @@ func NewWithConfigAndClient(config *ClientConfig, httpClient *http.Client) (*Con
 	}
 	if conn != nil {
 		// TODO: error handling results into `make test` failure.
-		conn.brokerSelector.init()
+		if err := conn.brokerSelector.init(); err != nil {
+			log.Errorf("Failed to initialize broker selector: %v", err)
+			return conn, err
+		}
 		return conn, nil
 	}
 	return nil, fmt.Errorf(

pinot/connectionFactory_test.go

@@ -13,7 +13,8 @@ func TestPinotClients(t *testing.T) {
 	assert.NotNil(t, pinotClient1)
 	assert.NotNil(t, pinotClient1.brokerSelector)
 	assert.NotNil(t, pinotClient1.transport)
-	assert.Nil(t, err)
+	// Since there is no zk setup, so an error will be raised
+	assert.NotNil(t, err)
 	pinotClient2, err := NewWithConfig(&ClientConfig{
 		ZkConfig: &ZookeeperConfig{
 			ZookeeperPath:     []string{"localhost:2181"},
@@ -27,11 +28,14 @@ func TestPinotClients(t *testing.T) {
 	assert.NotNil(t, pinotClient2)
 	assert.NotNil(t, pinotClient2.brokerSelector)
 	assert.NotNil(t, pinotClient2.transport)
-	assert.Nil(t, err)
+	// Since there is no zk setup, so an error will be raised
+	assert.NotNil(t, err)
 	pinotClient3, err := NewFromController("localhost:9000")
 	assert.NotNil(t, pinotClient3)
 	assert.NotNil(t, pinotClient3.brokerSelector)
 	assert.NotNil(t, pinotClient3.transport)
+	// Since there is no controller setup, so an error will be raised
+	assert.NotNil(t, err)
 	_, err = NewWithConfig(&ClientConfig{})
 	assert.NotNil(t, err)
 	assert.True(t, strings.Contains(err.Error(), "please specify"))

pinot/connection_test.go

@@ -113,8 +113,9 @@ func TestConnectionWithControllerBasedBrokerSelector(t *testing.T) {
 func TestSendingQueryWithTraceOpen(t *testing.T) {
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		var request map[string]string
-		json.NewDecoder(r.Body).Decode(&request)
+		err := json.NewDecoder(r.Body).Decode(&request)
 		assert.Equal(t, request["trace"], "true")
+		assert.Nil(t, err)
 	}))
 	defer ts.Close()
 	pinotClient, err := NewFromBrokerList([]string{ts.URL})
@@ -123,13 +124,16 @@ func TestSendingQueryWithTraceOpen(t *testing.T) {
 	assert.NotNil(t, pinotClient.transport)
 	assert.Nil(t, err)
 	pinotClient.OpenTrace()
-	pinotClient.ExecuteSQL("", "select teamID, count(*) as cnt, sum(homeRuns) as sum_homeRuns from baseballStats group by teamID limit 10")
+	resp, err := pinotClient.ExecuteSQL("", "select teamID, count(*) as cnt, sum(homeRuns) as sum_homeRuns from baseballStats group by teamID limit 10")
+	assert.Nil(t, resp)
+	assert.NotNil(t, err)
 }
 
 func TestSendingQueryWithTraceClose(t *testing.T) {
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		var request map[string]string
-		json.NewDecoder(r.Body).Decode(&request)
+		err := json.NewDecoder(r.Body).Decode(&request)
+		assert.Nil(t, err)
 		_, ok := request["trace"]
 		assert.False(t, ok)
 	}))
@@ -139,8 +143,12 @@ func TestSendingQueryWithTraceClose(t *testing.T) {
 	assert.NotNil(t, pinotClient.brokerSelector)
 	assert.NotNil(t, pinotClient.transport)
 	assert.Nil(t, err)
-	pinotClient.ExecuteSQL("", "select teamID, count(*) as cnt, sum(homeRuns) as sum_homeRuns from baseballStats group by teamID limit 10")
+	resp, err := pinotClient.ExecuteSQL("", "select teamID, count(*) as cnt, sum(homeRuns) as sum_homeRuns from baseballStats group by teamID limit 10")
+	assert.Nil(t, resp)
+	assert.NotNil(t, err)
 	pinotClient.OpenTrace()
 	pinotClient.CloseTrace()
-	pinotClient.ExecuteSQL("", "select teamID, count(*) as cnt, sum(homeRuns) as sum_homeRuns from baseballStats group by teamID limit 10")
+	resp, err = pinotClient.ExecuteSQL("", "select teamID, count(*) as cnt, sum(homeRuns) as sum_homeRuns from baseballStats group by teamID limit 10")
+	assert.Nil(t, resp)
+	assert.NotNil(t, err)
 }

pinot/controllerBasedBrokerSelector.go

@@ -21,34 +21,31 @@ var (
 	}
 )
 
+// HTTPClient is an interface for http.Client
 type HTTPClient interface {
 	Do(req *http.Request) (*http.Response, error)
 }
 
 type controllerBasedSelector struct {
 	client              HTTPClient
 	config              *ControllerConfig
-	controllerAPIReqUrl string
+	controllerAPIReqURL string
 	tableAwareBrokerSelector
 }
 
 func (s *controllerBasedSelector) init() error {
 	if s.config.UpdateFreqMs == 0 {
 		s.config.UpdateFreqMs = defaultUpdateFreqMs
 	}
-	u, err := getControllerRequestUrl(s.config.ControllerAddress)
+	var err error
+	s.controllerAPIReqURL, err = getControllerRequestURL(s.config.ControllerAddress)
 	if err != nil {
-		log.Error(err)
+		log.Errorf("An error occurred when parsing controller address: %v", err)
 		return err
 	}
-	s.controllerAPIReqUrl = u
 
-	err = s.updateBrokerData()
-	if err != nil {
-		log.Errorf(
-			"An error occurred when fetching broker data from controller API for the first time, Error: %v",
-			err,
-		)
+	if err = s.updateBrokerData(); err != nil {
+		log.Errorf("An error occurred when fetching broker data from controller API for the first time, Error: %v", err)
 		return err
 	}
 	go s.setupInterval()
@@ -73,7 +70,7 @@ func (s *controllerBasedSelector) setupInterval() {
 	}
 }
 
-func getControllerRequestUrl(controllerAddress string) (string, error) {
+func getControllerRequestURL(controllerAddress string) (string, error) {
 	tokenized := strings.Split(controllerAddress, "://")
 	addressWithScheme := controllerAddress
 	if len(tokenized) > 1 {
@@ -91,12 +88,9 @@ func getControllerRequestUrl(controllerAddress string) (string, error) {
 }
 
 func (s *controllerBasedSelector) createControllerRequest() (*http.Request, error) {
-	r, err := http.NewRequest("GET", s.controllerAPIReqUrl, nil)
+	r, err := http.NewRequest("GET", s.controllerAPIReqURL, nil)
 	if err != nil {
-		return r, fmt.Errorf(
-			"Caught exception when creating controller API request: %v",
-			err,
-		)
+		return r, fmt.Errorf("Caught exception when creating controller API request: %v", err)
 	}
 	for k, v := range controllerDefaultHTTPHeader {
 		r.Header.Add(k, v)
@@ -116,14 +110,18 @@ func (s *controllerBasedSelector) updateBrokerData() error {
 	if err != nil {
 		return fmt.Errorf("Got exceptions while sending controller API request: %v", err)
 	}
-	defer resp.Body.Close()
+	defer func() {
+		if err := resp.Body.Close(); err != nil {
+			log.Error("Unable to close response body. ", err)
+		}
+	}()
 	if resp.StatusCode == http.StatusOK {
 		bodyBytes, err := io.ReadAll(resp.Body)
 		if err != nil {
 			return fmt.Errorf("An error occurred when reading controller API response: %v", err)
 		}
 		var c controllerResponse
-		if err = decodeJsonWithNumber(bodyBytes, &c); err != nil {
+		if err = decodeJSONWithNumber(bodyBytes, &c); err != nil {
 			return fmt.Errorf("An error occurred when decoding controller API response: %v", err)
 		}
 		s.rwMux.Lock()