fix(cmd,pkg): refactor codebase to align with golanci-linter checks (…

…#506) Because - current codebase is not following some golang best practices This commit - refactor codebase to be more aligned with go philosophy and provides easier readability and maintainability resolves INS-3601
instill-ai · Feb 5, 2024 · b213812 · b213812
1 parent 476b118
commit b213812
Show file tree

Hide file tree

Showing 45 changed files with 600 additions and 556 deletions.
diff --git a/.golangci.yaml b/.golangci.yaml
@@ -28,6 +28,10 @@ linters-settings:
       - ifElseChain
       - octalLiteral
       - whyNoLint
+      - paramTypeCombine
+      - unnamedResult
+      - commentedOutCode
+      - tooManyResultsChecker
   gocyclo:
     min-complexity: 15
   gofmt:
@@ -52,7 +56,7 @@ linters-settings:
       - strings.SplitN
 
   govet:
-    check-shadowing: true
+    check-shadowing: false
     settings:
       printf:
         funcs:
@@ -75,6 +79,7 @@ linters-settings:
       - name: unexported-return
         disabled: true
       - name: unused-parameter
+        disabled: true
 
 linters:
   disable-all: true
@@ -84,35 +89,32 @@ linters:
     - dogsled
     # - dupl
     - errcheck
-    # - errorlint
+    - errorlint
     - exportloopref
     # - funlen
     - gocheckcompilerdirectives
     - gochecknoinits
     - goconst
-    # - gocritic
+    - gocritic
     # - gocyclo
-    # - gofmt
-    # - goimports
-    # - gomnd
+    - gofmt
+    - goimports
     - goprintffuncname
     # - gosec
     - gosimple
-    # - govet
+    - govet
     - ineffassign
-    # - lll
-    # - misspell
-    # - nakedret
-    # - noctx
+    - misspell
+    - nakedret
+    - noctx
     - nolintlint
-    # - revive
+    - revive
     - staticcheck
     - stylecheck
     - typecheck
-    # - unconvert
+    - unconvert
     - unparam
     - unused
-    # - whitespace
 
   # don't enable:
   # - asciicheck

diff --git a/cmd/init/main.go b/cmd/init/main.go
@@ -7,20 +7,20 @@ import (
 	"log"
 	"strings"
 
+	"github.com/gofrs/uuid"
 	"go.opentelemetry.io/otel"
 	"gorm.io/gorm"
 
-	"github.com/gofrs/uuid"
 	openfgaClient "github.com/openfga/go-sdk/client"
 
 	"github.com/instill-ai/model-backend/config"
 	"github.com/instill-ai/model-backend/pkg/acl"
 	"github.com/instill-ai/model-backend/pkg/datamodel"
-	"github.com/instill-ai/model-backend/pkg/logger"
 	"github.com/instill-ai/model-backend/pkg/repository"
 
 	database "github.com/instill-ai/model-backend/pkg/db"
 	databaseInit "github.com/instill-ai/model-backend/pkg/init"
+	custom_logger "github.com/instill-ai/model-backend/pkg/logger"
 	custom_otel "github.com/instill-ai/model-backend/pkg/logger/otel"
 	modelPB "github.com/instill-ai/protogen-go/model/model/v1alpha"
 )
@@ -66,12 +66,12 @@ func main() {
 	defer span.End()
 	defer cancel()
 
-	logger, _ := logger.GetZapLogger(ctx)
+	logger, _ := custom_logger.GetZapLogger(ctx)
 
 	db := database.GetConnection()
 	defer database.Close(db)
 
-	repository := repository.NewRepository(db)
+	repo := repository.NewRepository(db)
 
 	fgaClient, err := openfgaClient.NewSdkClient(&openfgaClient.ClientConfiguration{
 		ApiScheme: "http",
@@ -98,7 +98,7 @@ func main() {
 	var models []*datamodel.Model
 	pageToken := ""
 	for {
-		models, _, pageToken, err = repository.ListModelsAdmin(ctx, 100, pageToken, true, false)
+		models, _, pageToken, err = repo.ListModelsAdmin(ctx, 100, pageToken, true, false)
 		if err != nil {
 			panic(err)
 		}

diff --git a/cmd/main/main.go b/cmd/main/main.go
@@ -37,7 +37,6 @@ import (
 	"github.com/instill-ai/model-backend/pkg/constant"
 	"github.com/instill-ai/model-backend/pkg/external"
 	"github.com/instill-ai/model-backend/pkg/handler"
-	"github.com/instill-ai/model-backend/pkg/logger"
 	"github.com/instill-ai/model-backend/pkg/middleware"
 	"github.com/instill-ai/model-backend/pkg/ray"
 	"github.com/instill-ai/model-backend/pkg/repository"
@@ -49,6 +48,7 @@ import (
 	"github.com/instill-ai/x/zapadapter"
 
 	database "github.com/instill-ai/model-backend/pkg/db"
+	custom_logger "github.com/instill-ai/model-backend/pkg/logger"
 	custom_otel "github.com/instill-ai/model-backend/pkg/logger/otel"
 	mgmtPB "github.com/instill-ai/protogen-go/core/mgmt/v1beta"
 	modelPB "github.com/instill-ai/protogen-go/model/model/v1alpha"
@@ -95,7 +95,7 @@ func main() {
 	)
 	defer cancel()
 
-	logger, _ := logger.GetZapLogger(ctx)
+	logger, _ := custom_logger.GetZapLogger(ctx)
 	defer func() {
 		// can't handle the error due to https://github.com/uber-go/zap/issues/880
 		_ = logger.Sync()
@@ -151,17 +151,16 @@ func main() {
 		grpcServerOpts = append(grpcServerOpts, grpc.Creds(creds))
 	}
 
-	grpcServerOpts = append(grpcServerOpts, grpc.MaxRecvMsgSize(config.Config.Server.MaxDataSize*constant.MB))
-	grpcServerOpts = append(grpcServerOpts, grpc.MaxSendMsgSize(config.Config.Server.MaxDataSize*constant.MB))
+	grpcServerOpts = append(grpcServerOpts, grpc.MaxRecvMsgSize(config.Config.Server.MaxDataSize*constant.MB), grpc.MaxSendMsgSize(config.Config.Server.MaxDataSize*constant.MB))
 
 	privateGrpcS := grpc.NewServer(grpcServerOpts...)
 	reflection.Register(privateGrpcS)
 
 	publicGrpcS := grpc.NewServer(grpcServerOpts...)
 	reflection.Register(publicGrpcS)
 
-	triton := triton.NewTriton()
-	defer triton.Close()
+	tritonServer := triton.NewTriton()
+	defer tritonServer.Close()
 
 	rayService := ray.NewRay()
 	defer rayService.Close()
@@ -238,17 +237,17 @@ func main() {
 		panic(err)
 	}
 
-	repository := repository.NewRepository(db)
+	repo := repository.NewRepository(db)
 
-	service := service.NewService(repository, triton, mgmtPublicServiceClient, mgmtPrivateServiceClient, redisClient, temporalClient, controllerClient, rayService, &aclClient)
+	serv := service.NewService(repo, tritonServer, mgmtPublicServiceClient, mgmtPrivateServiceClient, redisClient, temporalClient, controllerClient, rayService, &aclClient)
 
 	modelPB.RegisterModelPublicServiceServer(
 		publicGrpcS,
-		handler.NewPublicHandler(ctx, service, triton, rayService))
+		handler.NewPublicHandler(ctx, serv, tritonServer, rayService))
 
 	modelPB.RegisterModelPrivateServiceServer(
 		privateGrpcS,
-		handler.NewPrivateHandler(ctx, service, triton))
+		handler.NewPrivateHandler(ctx, serv, tritonServer))
 
 	privateGwS := runtime.NewServeMux(
 		runtime.WithForwardResponseOption(middleware.HTTPResponseModifier),
@@ -271,12 +270,12 @@ func main() {
 	)
 
 	// Register custom route for  POST /v1alpha/models/{name=models/*}/trigger-multipart which makes model inference for REST multiple-part form-data
-	if err := publicGwS.HandlePath("POST", "/v1alpha/{path=users/*/models/*}/trigger-multipart", middleware.AppendCustomHeaderMiddleware(service, handler.HandleTriggerModelByUpload)); err != nil {
+	if err := publicGwS.HandlePath("POST", "/v1alpha/{path=users/*/models/*}/trigger-multipart", middleware.AppendCustomHeaderMiddleware(serv, handler.HandleTriggerModelByUpload)); err != nil {
 		panic(err)
 	}
 
 	// Register custom route for  POST /models/multipart which uploads model for REST multiple-part form-data
-	if err := publicGwS.HandlePath("POST", "/v1alpha/{parent=users/*}/models/multipart", middleware.AppendCustomHeaderMiddleware(service, handler.HandleCreateModelByMultiPartFormData)); err != nil {
+	if err := publicGwS.HandlePath("POST", "/v1alpha/{parent=users/*}/models/multipart", middleware.AppendCustomHeaderMiddleware(serv, handler.HandleCreateModelByMultiPartFormData)); err != nil {
 		panic(err)
 	}
 
@@ -296,7 +295,7 @@ func main() {
 		logger.Info("try to start usage reporter")
 		go func() {
 			for {
-				usg = usage.NewUsage(ctx, repository, mgmtPrivateServiceClient, redisClient, usageServiceClient, userUID)
+				usg = usage.NewUsage(ctx, repo, mgmtPrivateServiceClient, redisClient, usageServiceClient, userUID)
 				if usg != nil {
 					usg.StartReporter(ctx)
 					logger.Info("usage reporter started")

diff --git a/cmd/migration/main.go b/cmd/migration/main.go
@@ -12,7 +12,7 @@ import (
 	"github.com/instill-ai/model-backend/config"
 )
 
-func checkExist(databaseConfig config.DatabaseConfig) error {
+func checkExist(databaseConfig *config.DatabaseConfig) error {
 	db, err := sql.Open(
 		"postgres",
 		fmt.Sprintf("host=%s user=%s password=%s dbname=postgres port=%d sslmode=disable TimeZone=%s",
@@ -63,7 +63,7 @@ func checkExist(databaseConfig config.DatabaseConfig) error {
 
 	if !dbExist {
 		fmt.Printf("Create database %s\n", databaseConfig.Name)
-		if _, err := db.Exec(fmt.Sprintf("CREATE DATABASE \"%s\";", databaseConfig.Name)); err != nil {
+		if _, err := db.Exec(fmt.Sprintf("CREATE DATABASE %q;", databaseConfig.Name)); err != nil {
 			return err
 		}
 	}
@@ -76,7 +76,7 @@ func main() {
 
 	_ = config.Init()
 	databaseConfig := config.Config.Database
-	if err := checkExist(databaseConfig); err != nil {
+	if err := checkExist(&databaseConfig); err != nil {
 		panic(err)
 	}
 

diff --git a/cmd/model/main.go b/cmd/model/main.go
@@ -22,27 +22,26 @@ import (
 
 	"github.com/instill-ai/model-backend/config"
 	"github.com/instill-ai/model-backend/pkg/datamodel"
-	"github.com/instill-ai/model-backend/pkg/logger"
 	"github.com/instill-ai/model-backend/pkg/middleware"
 	"github.com/instill-ai/model-backend/pkg/utils"
 
+	custom_logger "github.com/instill-ai/model-backend/pkg/logger"
 	custom_otel "github.com/instill-ai/model-backend/pkg/logger/otel"
-	commonPB "github.com/instill-ai/protogen-go/common/task/v1alpha"
 	mgmtPB "github.com/instill-ai/protogen-go/core/mgmt/v1beta"
 	modelPB "github.com/instill-ai/protogen-go/model/model/v1alpha"
 )
 
 type ModelConfig struct {
-	ID              string                 `json:"id"`
-	Description     string                 `json:"description"`
-	Task            string                 `json:"task"`
-	ModelDefinition string                 `json:"model_definition"`
-	Configuration   map[string]interface{} `json:"configuration"`
+	ID              string         `json:"id"`
+	Description     string         `json:"description"`
+	Task            string         `json:"task"`
+	ModelDefinition string         `json:"model_definition"`
+	Configuration   map[string]any `json:"configuration"`
 }
 
-// InitMgmtPrivateServiceClient initialises a MgmtPrivateServiceClient instance
+// InitMgmtPrivateServiceClient initializes a MgmtPrivateServiceClient instance
 func InitMgmtPrivateServiceClient(ctx context.Context) (mgmtPB.MgmtPrivateServiceClient, *grpc.ClientConn) {
-	logger, _ := logger.GetZapLogger(ctx)
+	logger, _ := custom_logger.GetZapLogger(ctx)
 
 	var clientDialOpts grpc.DialOption
 	var creds credentials.TransportCredentials
@@ -65,9 +64,9 @@ func InitMgmtPrivateServiceClient(ctx context.Context) (mgmtPB.MgmtPrivateServic
 	return mgmtPB.NewMgmtPrivateServiceClient(clientConn), clientConn
 }
 
-// InitModelPublicServiceClient initialises a ModelServiceClient instance
+// InitModelPublicServiceClient initializes a ModelServiceClient instance
 func InitModelPublicServiceClient(ctx context.Context) (modelPB.ModelPublicServiceClient, *grpc.ClientConn) {
-	logger, _ := logger.GetZapLogger(ctx)
+	logger, _ := custom_logger.GetZapLogger(ctx)
 
 	var clientDialOpts grpc.DialOption
 	if config.Config.Server.HTTPS.Cert != "" && config.Config.Server.HTTPS.Key != "" {
@@ -111,7 +110,7 @@ func main() {
 		"main",
 	)
 
-	logger, _ := logger.GetZapLogger(ctx)
+	logger, _ := custom_logger.GetZapLogger(ctx)
 
 	defer func() {
 		// can't handle the error due to https://github.com/uber-go/zap/issues/880
@@ -206,7 +205,7 @@ func main() {
 				Model: &modelPB.Model{
 					Id:              modelConfig.ID,
 					Description:     &modelConfig.Description,
-					Task:            commonPB.Task(utils.Tasks[strings.ToUpper(modelConfig.Task)]),
+					Task:            utils.Tasks[strings.ToUpper(modelConfig.Task)],
 					ModelDefinition: modelConfig.ModelDefinition,
 					Configuration:   configuration,
 					Visibility:      modelPB.Model_VISIBILITY_PUBLIC,

diff --git a/cmd/worker/main.go b/cmd/worker/main.go
@@ -18,14 +18,14 @@ import (
 	"github.com/instill-ai/model-backend/config"
 	"github.com/instill-ai/model-backend/pkg/acl"
 	"github.com/instill-ai/model-backend/pkg/external"
-	"github.com/instill-ai/model-backend/pkg/logger"
 	"github.com/instill-ai/model-backend/pkg/ray"
 	"github.com/instill-ai/model-backend/pkg/repository"
 	"github.com/instill-ai/model-backend/pkg/triton"
 	"github.com/instill-ai/x/temporal"
 	"github.com/instill-ai/x/zapadapter"
 
 	database "github.com/instill-ai/model-backend/pkg/db"
+	custom_logger "github.com/instill-ai/model-backend/pkg/logger"
 	custom_otel "github.com/instill-ai/model-backend/pkg/logger/otel"
 	modelWorker "github.com/instill-ai/model-backend/pkg/worker"
 )
@@ -51,7 +51,7 @@ func main() {
 	)
 	defer cancel()
 
-	logger, _ := logger.GetZapLogger(ctx)
+	logger, _ := custom_logger.GetZapLogger(ctx)
 	defer func() {
 		// can't handle the error due to https://github.com/uber-go/zap/issues/880
 		_ = logger.Sync()
@@ -60,8 +60,8 @@ func main() {
 	db := database.GetSharedConnection()
 	defer database.Close(db)
 
-	triton := triton.NewTriton()
-	defer triton.Close()
+	tritonServer := triton.NewTriton()
+	defer tritonServer.Close()
 
 	controllerClient, controllerClientConn := external.InitControllerPrivateServiceClient(ctx)
 	defer controllerClientConn.Close()
@@ -132,7 +132,7 @@ func main() {
 		panic(err)
 	}
 
-	cw := modelWorker.NewWorker(repository.NewRepository(db), redisClient, triton, controllerClient, rayService, &aclClient)
+	cw := modelWorker.NewWorker(repository.NewRepository(db), redisClient, tritonServer, controllerClient, rayService, &aclClient)
 
 	w := worker.New(temporalClient, modelWorker.TaskQueue, worker.Options{})
 

diff --git a/config/config.go b/config/config.go
@@ -185,8 +185,8 @@ func Init() error {
 		log.Fatal(err.Error())
 	}
 
-	if err := k.Load(env.ProviderWithValue("CFG_", ".", func(s string, v string) (string, interface{}) {
-		key := strings.Replace(strings.ToLower(strings.TrimPrefix(s, "CFG_")), "_", ".", -1)
+	if err := k.Load(env.ProviderWithValue("CFG_", ".", func(s string, v string) (string, any) {
+		key := strings.ReplaceAll(strings.ToLower(strings.TrimPrefix(s, "CFG_")), "_", ".")
 		if strings.Contains(v, ",") {
 			return key, strings.Split(strings.TrimSpace(v), ",")
 		}
@@ -203,6 +203,7 @@ func Init() error {
 }
 
 // ValidateConfig is for custom validation rules for the configuration
-func ValidateConfig(cfg *AppConfig) error {
+// for future use
+func ValidateConfig(_ *AppConfig) error {
 	return nil
 }
diff --git a/pkg/acl/acl.go b/pkg/acl/acl.go
@@ -91,7 +91,7 @@ func (c *ACLClient) DeletePublicModelPermission(modelUID uuid.UUID) error {
 	return nil
 }
 
-func (c *ACLClient) SetModelPermission(modelUID uuid.UUID, user string, role string, enable bool) error {
+func (c *ACLClient) SetModelPermission(modelUID uuid.UUID, user, role string, enable bool) error {
 	var err error
 	options := openfgaClient.ClientWriteOptions{
 		AuthorizationModelId: c.authorizationModelID,
@@ -170,7 +170,6 @@ func (c *ACLClient) Purge(objectType string, objectUID uuid.UUID) error {
 }
 
 func (c *ACLClient) CheckPermission(objectType string, objectUID uuid.UUID, userType string, userUID uuid.UUID, role string) (bool, error) {
-
 	options := openfgaClient.ClientCheckOptions{
 		AuthorizationModelId: c.authorizationModelID,
 	}