Simplify change

Author: timvaillancourt

pkg/throttle/throttler.go

@@ -72,7 +72,6 @@ type Throttler struct {
 
 	nonLowPriorityAppRequestsThrottled *cache.Cache
 	httpClient                         *http.Client
-	vitessClient                       *vitess.Client
 }
 
 func NewThrottler() *Throttler {
@@ -321,13 +320,10 @@ func (throttler *Throttler) refreshMySQLInventory() error {
 			}
 
 			if !clusterSettings.VitessSettings.IsEmpty() {
-				if throttler.vitessClient == nil {
-					throttler.vitessClient = vitess.New()
-				}
 				log.Debugf("getting vitess data from %s", clusterSettings.VitessSettings.API)
 				keyspace := clusterSettings.VitessSettings.Keyspace
 				shard := clusterSettings.VitessSettings.Shard
-				tablets, err := throttler.vitessClient.ParseTablets(clusterSettings.VitessSettings.API, keyspace, shard, clusterSettings.VitessSettings.TimeoutSecs)
+				tablets, err := vitess.ParseTablets(clusterSettings.VitessSettings)
 				if err != nil {
 					return log.Errorf("Unable to get vitess hosts from %s, %s/%s: %+v", clusterSettings.VitessSettings.API, keyspace, shard, err)
 				}

pkg/vitess/api_client.go

@@ -8,7 +8,7 @@ import (
 	"strings"
 	"time"
 
-	"github.com/github/freno/pkg/base"
+	"github.com/github/freno/pkg/config"
 	"vitess.io/vitess/go/vt/proto/topodata"
 )
 
@@ -26,12 +26,16 @@ func (t Tablet) IsValidReplica() bool {
 	return t.Type == topodata.TabletType_REPLICA
 }
 
-func constructAPIURL(api string, keyspace string, shard string) (url string) {
-	api = strings.TrimRight(api, "/")
+var httpClient = http.Client{
+	Timeout: defaultTimeout,
+}
+
+func constructAPIURL(settings config.VitessConfigurationSettings) (url string) {
+	api := strings.TrimRight(settings.API, "/")
 	if !strings.HasSuffix(api, "/api") {
 		api = fmt.Sprintf("%s/api", api)
 	}
-	url = fmt.Sprintf("%s/keyspace/%s/tablets/%s", api, keyspace, shard)
+	url = fmt.Sprintf("%s/keyspace/%s/tablets/%s", api, settings.Keyspace, settings.Shard)
 
 	return url
 }
@@ -46,27 +50,17 @@ func filterReplicaTablets(tablets []Tablet) (replicas []Tablet) {
 	return replicas
 }
 
-// Client gathers info from the Vitess API
-type Client struct {
-	client *http.Client
-}
-
-// New returns a new Client
-func New() *Client {
-	return &Client{client: base.SetupHttpClient(0)}
-}
-
 // ParseTablets reads from vitess /api/ks_tablets/<keyspace>/[shard] and returns a
 // listing (mysql_hostname, mysql_port, type) of REPLICA tablets
-func (c *Client) ParseTablets(api, keyspace, shard string, timeoutSecs float64) (tablets []Tablet, err error) {
-	if timeoutSecs == 0 {
-		c.client.Timeout = defaultTimeout
+func ParseTablets(settings config.VitessConfigurationSettings) (tablets []Tablet, err error) {
+	if settings.TimeoutSecs == 0 {
+		httpClient.Timeout = defaultTimeout
 	} else {
-		c.client.Timeout = time.Duration(timeoutSecs) * time.Second
+		httpClient.Timeout = time.Duration(settings.TimeoutSecs) * time.Second
 	}
 
-	url := constructAPIURL(api, keyspace, shard)
-	resp, err := c.client.Get(url)
+	url := constructAPIURL(settings)
+	resp, err := httpClient.Get(url)
 	if err != nil {
 		return tablets, err
 	}

pkg/vitess/api_client_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/github/freno/pkg/config"
 	"vitess.io/vitess/go/vt/proto/topodata"
 )
 
@@ -50,10 +51,13 @@ func TestParseTablets(t *testing.T) {
 	}))
 	defer vitessApi.Close()
 
-	vtClient := New()
-
 	t.Run("success", func(t *testing.T) {
-		tablets, err := vtClient.ParseTablets(vitessApi.URL, "test", "00", 1)
+		tablets, err := ParseTablets(config.VitessConfigurationSettings{
+			API:         vitessApi.URL,
+			Keyspace:    "test",
+			Shard:       "00",
+			TimeoutSecs: 1.0,
+		})
 		if err != nil {
 			t.Fatalf("Expected no error, got %q", err)
 		}
@@ -66,13 +70,18 @@ func TestParseTablets(t *testing.T) {
 			t.Fatalf("Expected hostname %q, got %q", "replica", tablets[0].MysqlHostname)
 		}
 
-		if vtClient.client.Timeout != time.Second {
-			t.Fatalf("Expected vitess client timeout of %v, got %v", time.Second, vtClient.client.Timeout)
+		if httpClient.Timeout != time.Second {
+			t.Fatalf("Expected vitess client timeout of %v, got %v", time.Second, httpClient.Timeout)
 		}
 	})
 
 	t.Run("not-found", func(t *testing.T) {
-		tablets, err := vtClient.ParseTablets(vitessApi.URL, "not-found", "00", 0)
+		tablets, err := ParseTablets(config.VitessConfigurationSettings{
+			API:         vitessApi.URL,
+			Keyspace:    "not-found",
+			Shard:       "40-80",
+			TimeoutSecs: 0.0,
+		})
 		if err != nil {
 			t.Fatalf("Expected no error, got %q", err)
 		}
@@ -81,14 +90,19 @@ func TestParseTablets(t *testing.T) {
 			t.Fatalf("Expected 0 tablets, got %d", len(tablets))
 		}
 
-		if vtClient.client.Timeout != defaultTimeout {
-			t.Fatalf("Expected vitess client timeout of %v, got %v", defaultTimeout, vtClient.client.Timeout)
+		if httpClient.Timeout != defaultTimeout {
+			t.Fatalf("Expected vitess client timeout of %v, got %v", defaultTimeout, httpClient.Timeout)
 		}
 	})
 
 	t.Run("failed", func(t *testing.T) {
 		vitessApi.Close() // kill the mock vitess API
-		_, err := vtClient.ParseTablets(vitessApi.URL, "fail", "00", 0)
+		_, err := ParseTablets(config.VitessConfigurationSettings{
+			API:         vitessApi.URL,
+			Keyspace:    "fail",
+			Shard:       "00",
+			TimeoutSecs: 0.0,
+		})
 		if err == nil {
 			t.Fatal("Expected error, got nil")
 		}