Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add go fmt check #812

Merged
merged 3 commits into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .github/workflows/linters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,18 @@ permissions:
contents: read

jobs:
fmtcheck:
name: go fmt check
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-go@v5
with:
go-version: '1.23.0'
cache: false
- name: go fmt check
run: make fmtcheck

golangci:
name: golangci-lint
runs-on: ubuntu-latest
Expand Down
9 changes: 9 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ GIT_REVISION=`git rev-parse --short HEAD`
SPQR_VERSION=`git describe --tags --abbrev=0`
LDFLAGS=-ldflags "-X github.com/pg-sharding/spqr/pkg.GitRevision=${GIT_REVISION} -X github.com/pg-sharding/spqr/pkg.SpqrVersion=${SPQR_VERSION}"
GCFLAGS=-gcflags=all="-N -l"
GOFMT_FILES?=$$(find . -name '*.go' | grep -v vendor | grep -v yacc)

.PHONY : run
.DEFAULT_GOAL := deps
Expand Down Expand Up @@ -137,6 +138,14 @@ feature_test: clean_feature_test build_images
mkdir ./test/feature/logs
(cd test/feature; GODOG_FEATURE_DIR=generatedFeatures go test -timeout 150m)

####################### LINTERS #######################

fmt:
gofmt -w $(GOFMT_FILES)

fmtcheck:
@sh -c "'$(CURDIR)/script/gofmtcheck.sh'"

lint:
golangci-lint run --timeout=10m --out-format=colored-line-number --skip-dirs=yacc/console

Expand Down
4 changes: 2 additions & 2 deletions pkg/client/clientpool.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ func (c *PoolImpl) Shutdown() error {
// ClientPoolForeach iterates over all clients in the client pool and executes the provided function for each client.
//
// The provided function should have the following signature:
// func(clientID uint, client Client) error
//
// func(clientID uint, client Client) error
//
// Parameters:
// - f (func): The function to be executed for each client.
Expand All @@ -147,7 +148,6 @@ func (c *PoolImpl) ClientPoolForeach(cb func(client ClientInfo) error) error {
return nil
}


// NewClientPool creates a new instance of the PoolImpl struct, which implements the Pool interface.
//
// It initializes the pool map with an empty map and the mutex with a new sync.Mutex.
Expand Down
4 changes: 2 additions & 2 deletions pkg/config/balancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ var cfgBalancer Balancer
// LoadBalancerCfg loads the balancer configuration from the specified file path.
//
// Parameters:
// - cfgPath (string): The path of the configuration file.
// - cfgPath (string): The path of the configuration file.
//
// Returns:
// - error: an error if any occurred during the loading process.
Expand Down Expand Up @@ -70,6 +70,7 @@ func LoadBalancerCfg(cfgPath string) error {
// Parameters:
// - file (*os.File): the file containing the configuration data.
// - filepath (string): the path of the configuration file.
//
// Returns:
// - error: an error if any occurred during the initialization process.
func initBalancerConfig(file *os.File, filepath string) error {
Expand All @@ -86,7 +87,6 @@ func initBalancerConfig(file *os.File, filepath string) error {
return fmt.Errorf("unknown config format type: %s. Use .toml, .yaml or .json suffix in filename", filepath)
}


// BalancerConfig returns a pointer to the Balancer configuration.
//
// Returns:
Expand Down
1 change: 1 addition & 0 deletions pkg/config/coordinator.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ func LoadCoordinatorCfg(cfgPath string) error {
// Parameters:
// - file (*os.File): the file containing the configuration data.
// - filepath (string): the path of the configuration file.
//
// Returns:
// - error: an error if any occurred during the initialization process.
func initCoordinatorConfig(file *os.File, filepath string) error {
Expand Down
7 changes: 3 additions & 4 deletions pkg/decode/spqrql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,11 @@ import (
"github.com/stretchr/testify/assert"
)


// TestKeyRange is a unit test function for the KeyRange function.
//
// It tests the KeyRange function by creating a key range with the given parameters and
// asserting that the returned string matches the expected value.
//
//
// Parameters:
// - t (*testing.T): The testing object used for assertions.
//
Expand Down Expand Up @@ -48,7 +47,7 @@ func TestKeyRange(t *testing.T) {
//
// It tests the Distribution function by creating a distribution with the given parameters and
// asserting that the returned string matches the expected value.
//
//
// Parameters:
// - t (*testing.T): The testing object used for assertions.
//
Expand Down Expand Up @@ -90,7 +89,7 @@ func TestDistribution(t *testing.T) {
// TestDistributedRelation is a unit test function for the DistributedRelation function.
//
// It tests the DistributedRelation function by asserting that the returned string matches the expected string.
//
//
// Parameters:
// - t (*testing.T): The testing object used for assertions.
//
Expand Down
4 changes: 1 addition & 3 deletions pkg/meta/meta.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,8 @@ func processDrop(ctx context.Context, dstmt spqrparser.Statement, isCascade bool
if err != nil {
return err
}

ret := make([]string, 0)
if err != nil {
return err
}
for _, ds := range dss {
if ds.Id != "default" {
if len(ds.Relations) != 0 && !isCascade {
Expand Down
2 changes: 1 addition & 1 deletion pkg/pool/dbpool.go
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ func NewDBPool(mapping map[string]*config.Shard, startupParams *startup.StartupP
if err != nil {
return nil, err
}

return datashard.NewShard(shardKey, pgi, mapping[shardKey.Name], rule, startupParams)
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/pool/dbpool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func TestDbPoolOrderCaching(t *testing.T) {
clId := uint(1)

dbpool := pool.NewDBPoolFromMultiPool(map[string]*config.Shard{
key.Name: &config.Shard{
key.Name: {
Hosts: []string{
"h1",
"h2",
Expand Down Expand Up @@ -138,7 +138,7 @@ func TestDbPoolReadOnlyOrderDistribution(t *testing.T) {
clId := uint(1)

dbpool := pool.NewDBPoolFromMultiPool(map[string]*config.Shard{
key.Name: &config.Shard{
key.Name: {
Hosts: []string{
"h1",
"h2",
Expand Down
1 change: 0 additions & 1 deletion pkg/pool/shardpool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ func TestShardPoolConnectionAcquireDiscard(t *testing.T) {
assert.NoError(err)
assert.Equal(shardconn, conn)


statistics = shp.View()
assert.Equal(0, statistics.IdleConnections)
assert.Equal(0, statistics.QueueResidualSize)
Expand Down
2 changes: 1 addition & 1 deletion qdb/memqdb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ var mockShard = &qdb.Shard{
Hosts: []string{"host1", "host2"},
}
var mockKeyRange = &qdb.KeyRange{
LowerBound: [][]byte{[]byte{1, 2}},
LowerBound: [][]byte{{1, 2}},
ShardID: mockShard.ID,
KeyRangeID: "key_range_id",
}
Expand Down
56 changes: 3 additions & 53 deletions router/rulerouter/rulerouter.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"github.com/pg-sharding/spqr/pkg/pool"
"github.com/pg-sharding/spqr/pkg/shard"
"github.com/pg-sharding/spqr/pkg/spqrlog"
"github.com/pg-sharding/spqr/qdb"
rclient "github.com/pg-sharding/spqr/router/client"
"github.com/pg-sharding/spqr/router/port"
"github.com/pg-sharding/spqr/router/route"
Expand All @@ -31,16 +30,9 @@ type RuleRouter interface {
Shutdown() error
Reload(configPath string) error
PreRoute(conn net.Conn, pt port.RouterPortType) (rclient.RouterClient, error)
PreRouteInitializedClientAdm(cl rclient.RouterClient) (rclient.RouterClient, error)
ObsoleteRoute(key route.Key) error

AddDataShard(key qdb.ShardKey) error
AddWorldShard(key qdb.ShardKey) error
AddShardInstance(key qdb.ShardKey, host string)

CancelClient(csm *pgproto3.CancelRequest) error
AddClient(cl rclient.RouterClient)

CancelClient(csm *pgproto3.CancelRequest) error
ReleaseClient(cl rclient.RouterClient)

Config() *config.Router
Expand All @@ -61,21 +53,6 @@ type RuleRouterImpl struct {
notifier *notifier.Notifier
}

func (r *RuleRouterImpl) AddWorldShard(key qdb.ShardKey) error {
spqrlog.Zero.Info().
Str("shard name", key.Name).
Msg("added world datashard to rrouter")
return nil
}

func (r *RuleRouterImpl) AddShardInstance(key qdb.ShardKey, host string) {
panic("implement me")
}

func (r *RuleRouterImpl) AddDataShard(key qdb.ShardKey) error {
return nil
}

func (r *RuleRouterImpl) Shutdown() error {
return r.routePool.Shutdown()
}
Expand Down Expand Up @@ -185,7 +162,7 @@ func (r *RuleRouterImpl) PreRoute(conn net.Conn, pt port.RouterPortType) (rclien
}

if pt == port.ADMRouterPortType || cl.DB() == "spqr-console" {
return r.PreRouteInitializedClientAdm(cl)
return r.preRouteInitializedClientAdm(cl)
}

// match client to frontend rule
Expand Down Expand Up @@ -235,9 +212,6 @@ func (r *RuleRouterImpl) PreRoute(conn net.Conn, pt port.RouterPortType) (rclien
Uint("client", spqrlog.GetPointer(cl)).
Msg("client auth succeeded")

if err != nil {
return nil, err
}
if err := cl.AssignRoute(rt); err != nil {
return nil, err
}
Expand All @@ -248,7 +222,7 @@ func (r *RuleRouterImpl) PreRoute(conn net.Conn, pt port.RouterPortType) (rclien
}

// TODO : unit tests
func (r *RuleRouterImpl) PreRouteInitializedClientAdm(cl rclient.RouterClient) (rclient.RouterClient, error) {
func (r *RuleRouterImpl) preRouteInitializedClientAdm(cl rclient.RouterClient) (rclient.RouterClient, error) {
key := *route.NewRouteKey(cl.Usr(), cl.DB())
frRule, err := r.rmgr.MatchKeyFrontend(key)
if err != nil {
Expand All @@ -274,30 +248,6 @@ func (r *RuleRouterImpl) PreRouteInitializedClientAdm(cl rclient.RouterClient) (
return cl, nil
}

// TODO : unit tests
func (r *RuleRouterImpl) ListShards() []string {
var ret []string

for _, sh := range r.rcfg.ShardMapping {
ret = append(ret, sh.Hosts[0])
}

return ret
}

// TODO : unit tests
func (r *RuleRouterImpl) ObsoleteRoute(key route.Key) error {
rt := r.routePool.Obsolete(key)

if err := rt.NofityClients(func(cl client.ClientInfo) error {
return nil
}); err != nil {
return err
}

return nil
}

// TODO : unit tests
func (r *RuleRouterImpl) Config() *config.Router {
r.mu.Lock()
Expand Down
15 changes: 15 additions & 0 deletions script/gofmtcheck.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#!/usr/bin/env bash

# Check gofmt
echo "==> Checking that code complies with gofmt requirements..."
gofmt_files=$(gofmt -l `find . -name '*.go' | grep -v vendor | grep -v yacc`)
if [[ -n ${gofmt_files} ]]; then
echo 'gofmt needs running on the following files:'
echo "${gofmt_files}"
echo "You can use the command: \`make fmt\` to reformat code."
exit 1
fi
echo "OK"


exit 0
Loading