Skip to content

Commit

Permalink
clean up lint and add it to the pre-commit hook (#167)
Browse files Browse the repository at this point in the history
  • Loading branch information
pmenglund committed Oct 25, 2023
1 parent 77cb75b commit f95670b
Show file tree
Hide file tree
Showing 30 changed files with 151 additions and 85 deletions.
7 changes: 1 addition & 6 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ run:
concurrency: 4
timeout: 1m
skip-dirs:
- client
- models
- openapi

linters-settings:
gci:
Expand All @@ -16,8 +15,6 @@ linters-settings:

linters:
enable:
- deadcode
- depguard
- dogsled
- errcheck
- exportloopref
Expand All @@ -42,11 +39,9 @@ linters:
- nolintlint
- rowserrcheck
- staticcheck
- structcheck
- stylecheck
- typecheck
- unconvert
- unparam
- unused
- varcheck
- whitespace
30 changes: 30 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
repos:
- repo: local
hooks:
- id: gofmt
name: "go fmt"
entry: gofmt
language: system
'types_or': [go]
args: ["-w", "-s"]
require_serial: false
additional_dependencies: []
minimum_pre_commit_version: 2.9.2
- id: govet
name: "go vet"
entry: go
language: system
'types_or': [go]
args: ["vet", "./..."]
require_serial: false
pass_filenames: false
additional_dependencies: []
minimum_pre_commit_version: 2.9.2
- id: golangci-lint
name: golangci-lint
description: Fast linters runner for Go. Note that only modified files are linted, so linters like 'unused' that need to scan all files won't work as expected.
entry: golangci-lint run --new-from-rev HEAD --fix
types: [go]
language: golang
require_serial: true
pass_filenames: false
2 changes: 1 addition & 1 deletion aliases_test.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
package rockset_test

import (
"github.com/rockset/rockset-go-client/internal/test"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"

"github.com/rockset/rockset-go-client"
"github.com/rockset/rockset-go-client/internal/test"
"github.com/rockset/rockset-go-client/option"
)

Expand Down
6 changes: 4 additions & 2 deletions collections.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ func (rc *RockClient) DeleteCollection(ctx context.Context, workspace, name stri

// UpdateCollection updates a collection. Only the option.WithCollectionDescription and
// option.WithIngestTransformation can be used, and any other option will return an error.
func (rc *RockClient) UpdateCollection(ctx context.Context, workspace, name string, options ...option.CollectionOption) (openapi.Collection, error) {
func (rc *RockClient) UpdateCollection(ctx context.Context, workspace, name string,
options ...option.CollectionOption) (openapi.Collection, error) {
var err error
var httpResp *http.Response
var resp *openapi.GetCollectionResponse
Expand Down Expand Up @@ -167,7 +168,8 @@ func (rc *RockClient) UpdateCollection(ctx context.Context, workspace, name stri
// ),
// option.WithS3Prefix("cities.csv"),
// ),
// option.WithKafkaSource("kafka-integration-name", "topic", option.KafkaStartingOffsetEarliest, option.WithJSONFormat(),
// option.WithKafkaSource("kafka-integration-name", "topic",
// option.KafkaStartingOffsetEarliest, option.WithJSONFormat(),
// option.WithKafkaSourceV3(),
// ),
// option.WithCollectionRetention(time.Hour),
Expand Down
3 changes: 3 additions & 0 deletions collections_kafka_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,13 @@ func (s *KafkaIntegrationSuite) TestKafka() {
testKafka(ctx, s.T(), s.rc, s.kc)
}

//nolint:funlen
func (s *KafkaIntegrationSuite) SetupSuite() {
var err error
ctx := test.Context()

_, err = s.rc.CreateWorkspace(ctx, s.kc.workspace)
s.Require().NoError(err)
s.dockerPool, err = dockertest.NewPool("")
s.Require().NoError(err)

Expand All @@ -86,6 +88,7 @@ func (s *KafkaIntegrationSuite) SetupSuite() {
s.Require().NoError(err, "could not connect zookeeper")
defer conn.Close()

//nolint:exhaustive
retryFn := func() error {
switch conn.State() {
case zk.StateHasSession, zk.StateConnected:
Expand Down
3 changes: 2 additions & 1 deletion errors/rockset.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ func New(err error) Error {
}

// NewWithStatusCode wraps err in an Error that provides better error messages than the openapi.GenericOpenAPIError,
// and can be retried if the HTTP response StatusCode is in RetryableErrors. If err is nil, NewWithStatusCode() returns nil.
// and can be retried if the HTTP response StatusCode is in RetryableErrors. If err is nil,
// NewWithStatusCode() returns nil.
func NewWithStatusCode(err error, response *http.Response) error {
if err == nil {
return nil
Expand Down
2 changes: 1 addition & 1 deletion errors/rockset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package errors_test

import (
"errors"
"github.com/rockset/rockset-go-client/openapi"
"net/http"
"testing"

Expand All @@ -11,6 +10,7 @@ import (

rockerr "github.com/rockset/rockset-go-client/errors"
"github.com/rockset/rockset-go-client/internal/test"
"github.com/rockset/rockset-go-client/openapi"
)

func TestError_IsNotFoundError(t *testing.T) {
Expand Down
3 changes: 2 additions & 1 deletion ha/ha.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ func New(client ...Querier) *Client {
}

// Query executes a SQL query in parallel, and returns the first response.
func (ha *Client) Query(ctx context.Context, query string, options ...option.QueryOption) (openapi.QueryResponse, []error) {
func (ha *Client) Query(ctx context.Context, query string,
options ...option.QueryOption) (openapi.QueryResponse, []error) {
log := zerolog.Ctx(ctx)

var wg sync.WaitGroup
Expand Down
5 changes: 4 additions & 1 deletion internal/test/string.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const PersistentAlias = "getalias"

const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"

//nolint:gosec
var seededRand = rand.New(rand.NewSource(time.Now().UnixNano()))

// StringWithCharset creates a random string with length and charset
Expand All @@ -32,6 +33,8 @@ func RandomString(length int) string {
return stringWithCharset(length, charset)
}

const randomNameLength = 6

func RandomName(prefix string) string {
num, found := os.LookupEnv(buildNum)
if !found {
Expand All @@ -42,7 +45,7 @@ func RandomName(prefix string) string {
}
}

return fmt.Sprintf("go_%s_%s_%s", num, prefix, RandomString(6))
return fmt.Sprintf("go_%s_%s_%s", num, prefix, RandomString(randomNameLength))
}

func Description() string {
Expand Down
28 changes: 20 additions & 8 deletions kafka_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,17 @@ import (
"context"
"encoding/json"
"fmt"
"github.com/rockset/rockset-go-client"
"github.com/rockset/rockset-go-client/internal/test"
"github.com/rockset/rockset-go-client/option"
"github.com/stretchr/testify/require"
"io"
"log"
"net/http"
"os"
"testing"

"github.com/stretchr/testify/require"

"github.com/rockset/rockset-go-client"
"github.com/rockset/rockset-go-client/internal/test"
"github.com/rockset/rockset-go-client/option"
)

type kafkaConfig struct {
Expand Down Expand Up @@ -103,7 +105,12 @@ func createConnector(url, name string, cfg ConnectorConfig) error {
}

c := http.Client{}
resp, err := c.Post(url, "application/json", bytes.NewReader(body))
req, err := http.NewRequestWithContext(context.TODO(), http.MethodPost, url, bytes.NewReader(body))
if err != nil {
return err
}

resp, err := c.Do(req)
if err != nil {
return err
}
Expand All @@ -121,7 +128,7 @@ func createConnector(url, name string, cfg ConnectorConfig) error {
}
func deleteConnector(url, name string) error {
c := http.Client{}
r, err := http.NewRequest(http.MethodDelete, fmt.Sprintf("%s/%s", url, name), nil)
r, err := http.NewRequestWithContext(context.TODO(), http.MethodDelete, fmt.Sprintf("%s/%s", url, name), nil)
if err != nil {
return err
}
Expand All @@ -146,7 +153,11 @@ func deleteConnector(url, name string) error {
func waitForKafkaConnect(t *testing.T, url string) func() error {
return func() error {
c := http.Client{}
resp, err := c.Get(url)
req, err := http.NewRequestWithContext(context.TODO(), http.MethodGet, url, nil)
if err != nil {
return err
}
resp, err := c.Do(req)
if err != nil {
return err
}
Expand Down Expand Up @@ -224,7 +235,8 @@ func connectParams(prefix, bootstrapServers, username, password string) []string
fmt.Sprintf("CONNECT_%sSSL_ENDPOINT_IDENTIFICATION_ALGORITHM=https", prefix),
fmt.Sprintf("CONNECT_%sSECURITY_PROTOCOL=SASL_SSL", prefix),
fmt.Sprintf("CONNECT_%sSASL_MECHANISM=PLAIN", prefix),
fmt.Sprintf(`CONNECT_%sSASL_JAAS_CONFIG=org.apache.kafka.common.security.plain.PlainLoginModule required username="%s" password="%s";`, prefix, username, password),
fmt.Sprintf(`CONNECT_%sSASL_JAAS_CONFIG=org.apache.kafka.common.security.plain.PlainLoginModule `+
`required username="%s" password="%s";`, prefix, username, password),
fmt.Sprintf("CONNECT_%sREQUEST_TIMEOUT_MS=20000", prefix),
fmt.Sprintf("CONNECT_%sRETRY_BACKOFF_MS=500", prefix),
}
Expand Down
6 changes: 4 additions & 2 deletions option/actions_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package option_test

import (
"github.com/rockset/rockset-go-client/option"
"github.com/stretchr/testify/assert"
"testing"

"github.com/stretchr/testify/assert"

"github.com/rockset/rockset-go-client/option"
)

func TestGetGlobalAction(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion organizations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/rockset/rockset-go-client/internal/test"
)

Expand Down
4 changes: 2 additions & 2 deletions queries_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ import (

// for anyone poking around in the code, rockset.sleep() only works for this test org as no sane person would want
// to add a sleep in their query
const slowQuery = `script {{{ import * as rockset from "/rockset"; export function delay(x) { rockset.sleep(x); return x; } }}} select _script.delay(2000, q) from unnest([1] as q)`
const slowQuery = `script {{{ import * as rockset from "/rockset"; export function delay(x) ` +
`{ rockset.sleep(x); return x; } }}} select _script.delay(2000, q) from unnest([1] as q)`

type QueryIntegrationSuite struct {
suite.Suite
rc *rockset.RockClient
}

func TestQueryIntegration(t *testing.T) {

s := QueryIntegrationSuite{}
suite.Run(t, &s)
}
Expand Down
2 changes: 2 additions & 0 deletions retry/exponential.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ func (r Exponential) Retry(ctx context.Context, retryFn Func) error {
return ctx.Err()
case t := <-t.C:
log.Trace().Str("t", t.String()).Msg("wait time")
//nolint:gosec
var jitter = time.Duration(jitterFraction*rand.Float64()) * waitInterval
waitInterval *= 2
waitInterval += jitter
Expand Down Expand Up @@ -135,6 +136,7 @@ func (r Exponential) RetryWithCheck(ctx context.Context, checkFn CheckFn) error
return ctx.Err()
case t := <-t.C:
log.Trace().Str("t", t.String()).Msg("wait time")
//nolint:gosec
var jitter = time.Duration(jitterFraction*rand.Float64()) * waitInterval
waitInterval *= 2
waitInterval += jitter
Expand Down
1 change: 0 additions & 1 deletion retry/exponential_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ type ExponentialRetrySuite struct {
}

func TestExponentialRetrySuite(t *testing.T) {

suite.Run(t, &ExponentialRetrySuite{})
}

Expand Down
14 changes: 7 additions & 7 deletions rockset.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"encoding/json"
"errors"
"fmt"
"github.com/rockset/rockset-go-client/wait"
"io"
"net/http"
"net/http/httputil"
Expand All @@ -17,6 +16,7 @@ import (

"github.com/rockset/rockset-go-client/openapi"
"github.com/rockset/rockset-go-client/retry"
"github.com/rockset/rockset-go-client/wait"
)

const (
Expand Down Expand Up @@ -82,7 +82,7 @@ func NewClient(options ...RockOption) (*RockClient, error) {
}

if rc.APIServer == "" {
return nil, NoAPIServerErr
return nil, ErrNoAPIServer
}

u, err := url.Parse(rc.APIServer)
Expand All @@ -102,9 +102,9 @@ func NewClient(options ...RockOption) (*RockClient, error) {
cfg.Scheme = "https" // we do not allow setting the scheme from the URL as we only support HTTPS

if rc.APIKey == "" && rc.Token == "" {
return nil, NoAPICredentialsErr
return nil, ErrNoAPICredentials
} else if rc.APIKey != "" && rc.Token != "" {
return nil, DuplicateCredentialsErr
return nil, ErrDuplicateCredentials
} else if rc.APIKey != "" {
cfg.AddDefaultHeader("Authorization", "apikey "+rc.APIKey)
} else {
Expand All @@ -122,9 +122,9 @@ func NewClient(options ...RockOption) (*RockClient, error) {
}

var (
NoAPICredentialsErr = errors.New("no API credentials provided")
DuplicateCredentialsErr = errors.New("duplicate API credentials provided")
NoAPIServerErr = errors.New("no API server provided")
ErrNoAPICredentials = errors.New("no API credentials provided")
ErrDuplicateCredentials = errors.New("duplicate API credentials provided")
ErrNoAPIServer = errors.New("no API server provided")
)

// RockOption is the type for RockClient options.
Expand Down
6 changes: 3 additions & 3 deletions rockset_test.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
package rockset_test

import (
"github.com/stretchr/testify/suite"
"os"
"testing"

"github.com/rs/zerolog"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"

"github.com/rockset/rockset-go-client"
"github.com/rockset/rockset-go-client/internal/test"
Expand Down Expand Up @@ -58,12 +58,12 @@ func (s *RockOptionSuite) SetupTest() {

func (s *RockOptionSuite) TestMissingCreds() {
_, err := rockset.NewClient(rockset.WithAPIServer("server"))
s.ErrorIs(err, rockset.NoAPICredentialsErr)
s.ErrorIs(err, rockset.ErrNoAPICredentials)
}

func (s *RockOptionSuite) TestMissingServer() {
_, err := rockset.NewClient(rockset.WithAPIKey("key"))
s.ErrorIs(err, rockset.NoAPIServerErr)
s.ErrorIs(err, rockset.ErrNoAPIServer)
}

func (s *RockOptionSuite) TestLastCredWins() {
Expand Down
Loading

0 comments on commit f95670b

Please sign in to comment.