Skip to content

Commit

Permalink
Add stricter formatters (G-Research#155)
Browse files Browse the repository at this point in the history
* Add stricter formatters
* Apply stricter formatting
  • Loading branch information
jgiannuzzi authored Jul 6, 2023
1 parent 94b3862 commit 2c11bc2
Show file tree
Hide file tree
Showing 30 changed files with 167 additions and 107 deletions.
6 changes: 4 additions & 2 deletions .devcontainer/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ RUN apt-get update && export DEBIAN_FRONTEND=noninteractive \

# [Optional] Uncomment the next lines to use go get to install anything else you need
USER vscode
RUN go install github.com/vektra/mockery/v2@v2.30.1
RUN go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.53.3
RUN go install github.com/vektra/mockery/v2@v2.30.16 \
&& go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.53.3 \
&& go install golang.org/x/tools/cmd/goimports@v0.11.0 \
&& go install mvdan.cc/gofumpt@v0.5.0
USER root

# [Optional] Uncomment this line to install global node packages.
Expand Down
21 changes: 18 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,26 @@ jobs:
id: tags
run: echo tags=$(jq -r '."go.buildTags"' .vscode/settings.json) >> $GITHUB_OUTPUT

- name: Download formatters
run: |
go install golang.org/x/tools/cmd/goimports@v0.11.0
go install mvdan.cc/gofumpt@v0.5.0
- name: Check formatting
run: gofmt -l .
run: |
unformatted=$(
gofumpt -l .
goimports -l --local github.com/G-Research/fasttrackml .
)
if [ -n "$unformatted" ]; then
for file in $unformatted; do
echo "::error file=$file::$file is not formatted properly (hint: run \"make go-format\" to fix this)"
done
exit 1
fi
- name: Download mockery
run: go install github.com/vektra/mockery/v2@v2.20.0
run: go install github.com/vektra/mockery/v2@v2.30.16

- name: Generate mocks
run: make mocks-generate
Expand All @@ -60,7 +75,7 @@ jobs:
go-version: "1.20"

- name: Download mockery
run: go install github.com/vektra/mockery/v2@v2.20.0
run: go install github.com/vektra/mockery/v2@v2.30.16

- name: Generate mocks
run: make mocks-generate
Expand Down
7 changes: 5 additions & 2 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
{
"go.testTags": "netgo osusergo sqlite_foreign_keys sqlite_math_functions sqlite_omit_load_extension sqlite_unlock_notify sqlite_vacuum_incr",
"go.buildTags": "netgo osusergo sqlite_foreign_keys sqlite_math_functions sqlite_omit_load_extension sqlite_unlock_notify sqlite_vacuum_incr"
"go.buildTags": "netgo osusergo sqlite_foreign_keys sqlite_math_functions sqlite_omit_load_extension sqlite_unlock_notify sqlite_vacuum_incr",
"gopls": {
"formatting.local": "github.com/G-Research/fasttrackml",
"formatting.gofumpt": true
}
}
6 changes: 6 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ go-build: ## build app binary.
@echo '>>> Building go binary.'
@go build -ldflags="-linkmode external -extldflags -static -s -w" -tags "$$(jq -r '."go.buildTags"' .vscode/settings.json)" -o $(APP) ./main.go

.PHONY: go-format
go-format: ## format go code.
@echo '>>> Formatting go code.'
@gofumpt -w .
@goimports -w -local github.com/G-Research/fasttrackml .

#
# Tests targets.
#
Expand Down
4 changes: 2 additions & 2 deletions main.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package main

import (
"github.com/G-Research/fasttrackml/pkg/cmd"

log "github.com/sirupsen/logrus"

"github.com/G-Research/fasttrackml/pkg/cmd"
)

func main() {
Expand Down
4 changes: 2 additions & 2 deletions pkg/api/aim/apps.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ package aim
import (
"fmt"

"github.com/G-Research/fasttrackml/pkg/database"

"github.com/gofiber/fiber/v2"
"github.com/google/uuid"
"gorm.io/gorm"

"github.com/G-Research/fasttrackml/pkg/database"
)

func GetApps(c *fiber.Ctx) error {
Expand Down
5 changes: 2 additions & 3 deletions pkg/api/aim/dashboards.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@ package aim
import (
"fmt"

"github.com/G-Research/fasttrackml/pkg/database"

"github.com/gofiber/fiber/v2"
"github.com/google/uuid"
"gorm.io/gorm"

"github.com/gofiber/fiber/v2"
"github.com/G-Research/fasttrackml/pkg/database"
)

func GetDashboards(c *fiber.Ctx) error {
Expand Down
6 changes: 4 additions & 2 deletions pkg/api/aim/encoding/encoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ import (
"reflect"
)

type arrayFlag struct{}
type objectFlag struct{}
type (
arrayFlag struct{}
objectFlag struct{}
)

var pathSentinel = []byte{0xfe}

Expand Down
4 changes: 2 additions & 2 deletions pkg/api/aim/experiments.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ import (
"strconv"
"time"

"github.com/G-Research/fasttrackml/pkg/database"

"github.com/gofiber/fiber/v2"
"gorm.io/gorm"

"github.com/G-Research/fasttrackml/pkg/database"
)

func GetExperiments(c *fiber.Ctx) error {
Expand Down
4 changes: 2 additions & 2 deletions pkg/api/aim/projects.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import (
"strconv"
"time"

"github.com/G-Research/fasttrackml/pkg/database"

"github.com/gofiber/fiber/v2"

"github.com/G-Research/fasttrackml/pkg/database"
)

func GetProject(c *fiber.Ctx) error {
Expand Down
8 changes: 4 additions & 4 deletions pkg/api/aim/runs.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,16 @@ import (
"strings"
"time"

"github.com/gofiber/fiber/v2"
log "github.com/sirupsen/logrus"
"gorm.io/gorm"

"github.com/G-Research/fasttrackml/pkg/api/aim/encoding"
"github.com/G-Research/fasttrackml/pkg/api/aim/query"
"github.com/G-Research/fasttrackml/pkg/api/aim/request"
"github.com/G-Research/fasttrackml/pkg/api/mlflow/dao/models"
"github.com/G-Research/fasttrackml/pkg/api/mlflow/dao/repositories"
"github.com/G-Research/fasttrackml/pkg/database"

"github.com/gofiber/fiber/v2"
log "github.com/sirupsen/logrus"
"gorm.io/gorm"
)

// validation rule for SearchMetrics
Expand Down
46 changes: 23 additions & 23 deletions pkg/api/aim/runs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,59 +7,59 @@ import (
)

func Test_validateMetricNamePresent(t *testing.T) {
tests := []struct{
name string
query string
tests := []struct {
name string
query string
wantResult bool
}{
{
name: "QueryWithMetricName",
query: `(run.active == true) and ((metric.name == "accuracy"))`,
name: "QueryWithMetricName",
query: `(run.active == true) and ((metric.name == "accuracy"))`,
wantResult: true,
},
{
name: "QueryWithMetricNameNoSpaces",
query: `(run.active == true) and ((metric.name=="accuracy"))`,
name: "QueryWithMetricNameNoSpaces",
query: `(run.active == true) and ((metric.name=="accuracy"))`,
wantResult: true,
},
{
name: "QueryWithStringSyntax",
query: `(run.active == true) and (metric.name.startswith("acc"))`,
name: "QueryWithStringSyntax",
query: `(run.active == true) and (metric.name.startswith("acc"))`,
wantResult: true,
},
{
name: "QueryWithInSyntax",
query: `(run.active == true) and ("accuracy" in metric.name)`,
name: "QueryWithInSyntax",
query: `(run.active == true) and ("accuracy" in metric.name)`,
wantResult: true,
},
{
name: "QueryWithMetricNameAtEnd",
query: `(run.active == true) and "accuracy" in metric.name`,
name: "QueryWithMetricNameAtEnd",
query: `(run.active == true) and "accuracy" in metric.name`,
wantResult: true,
},
{
name: "QueryWithoutMetricName",
query: `(run.active == true)`,
name: "QueryWithoutMetricName",
query: `(run.active == true)`,
wantResult: false,
},
{
name: "QueryWithoutDot",
query: `(run.active == true) and (metricname === "accuracy")`,
name: "QueryWithoutDot",
query: `(run.active == true) and (metricname === "accuracy")`,
wantResult: false,
},
{
name: "QueryWithOrCharacter",
query: `(run.active == true) and (metric.name| === "accuracy")`,
name: "QueryWithOrCharacter",
query: `(run.active == true) and (metric.name| === "accuracy")`,
wantResult: false,
},
{
name: "QueryWithTrickyDictKey",
query: `(run.active == true) and (run.tags["metric.name"] == "foo")`,
name: "QueryWithTrickyDictKey",
query: `(run.active == true) and (run.tags["metric.name"] == "foo")`,
wantResult: false,
},
{
name: "QueryWithTrickyDictKeyAndMetricName",
query: `(run.active == true) and (run.tags["mymetric.name"] == "foo") and (metric.name == "accuracy")`,
name: "QueryWithTrickyDictKeyAndMetricName",
query: `(run.active == true) and (run.tags["mymetric.name"] == "foo") and (metric.name == "accuracy")`,
wantResult: true,
},
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/api/mlflow/api/response/metric_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
)

func TestNewMetricHistoryResponse_Ok(t *testing.T) {
var testData = []struct {
testData := []struct {
name string
metrics []models.Metric
expectedResponse *GetMetricHistoryResponse
Expand Down Expand Up @@ -73,7 +73,7 @@ func TestNewMetricHistoryResponse_Ok(t *testing.T) {
}

func TestNewMetricHistoryBulkResponse_Ok(t *testing.T) {
var testData = []struct {
testData := []struct {
name string
metrics []models.Metric
expectedResponse *GetMetricHistoryBulkResponse
Expand Down
2 changes: 1 addition & 1 deletion pkg/api/mlflow/api/response/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
)

func TestNewRunPartialResponse(t *testing.T) {
var testData = []struct {
testData := []struct {
name string
run *models.Run
expectedResponse *RunPartialResponse
Expand Down
2 changes: 1 addition & 1 deletion pkg/api/mlflow/controller/metric.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"fmt"
"time"

"github.com/G-Research/fasttrackml/pkg/database"
"github.com/apache/arrow/go/v11/arrow"
"github.com/apache/arrow/go/v11/arrow/array"
"github.com/apache/arrow/go/v11/arrow/ipc"
Expand All @@ -16,6 +15,7 @@ import (
"github.com/G-Research/fasttrackml/pkg/api/mlflow/api"
"github.com/G-Research/fasttrackml/pkg/api/mlflow/api/request"
"github.com/G-Research/fasttrackml/pkg/api/mlflow/api/response"
"github.com/G-Research/fasttrackml/pkg/database"
)

// GetMetricHistory handles `GET /metrics/get-history` endpoint.
Expand Down
1 change: 0 additions & 1 deletion pkg/api/mlflow/dao/convertors/log_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ func TestConvertLogBatchRequestToDBModel_Ok(t *testing.T) {
}},
Params: []request.ParamPartialRequest{
{

Key: "key",
Value: "value",
},
Expand Down
3 changes: 1 addition & 2 deletions pkg/api/mlflow/dao/convertors/run.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
package convertors

import (
"fmt"

"database/sql"
"fmt"

"github.com/G-Research/fasttrackml/pkg/api/mlflow/api/request"
"github.com/G-Research/fasttrackml/pkg/api/mlflow/dao/models"
Expand Down
3 changes: 1 addition & 2 deletions pkg/api/mlflow/dao/convertors/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ import (
)

func TestConvertCreateRunRequestToDBModel(t *testing.T) {

var testData = []struct {
testData := []struct {
name string
req *request.CreateRunRequest
result func(run *models.Run)
Expand Down
3 changes: 0 additions & 3 deletions pkg/api/mlflow/dao/repositories/experiment.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,7 @@ func (r ExperimentRepository) GetByName(ctx context.Context, name string) (*mode

// Update updates existing models.Experiment entity.
func (r ExperimentRepository) Update(ctx context.Context, experiment *models.Experiment) error {

if err := r.db.Transaction(func(tx *gorm.DB) error {

if err := r.db.WithContext(ctx).Model(&experiment).Updates(experiment).Error; err != nil {
return eris.Wrapf(err, "error updating experiment with id: %d", *experiment.ID)
}
Expand All @@ -100,7 +98,6 @@ func (r ExperimentRepository) Update(ctx context.Context, experiment *models.Exp
}
}
return nil

}); err != nil {
return err
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/api/mlflow/service/artifact/validators_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ func TestValidateListArtifactsRequest_Ok(t *testing.T) {
})
assert.Nil(t, err)
}

func TestValidateListArtifactsRequest_Error(t *testing.T) {
var testData = []struct {
testData := []struct {
name string
error *api.ErrorResponse
request *request.ListArtifactsRequest
Expand Down
1 change: 0 additions & 1 deletion pkg/api/mlflow/service/experiment/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,6 @@ func (s Service) SearchExperiments(
),
).Decode(&token); err != nil {
return nil, 0, 0, api.NewInvalidParameterValueError("invalid page_token '%s': %s", req.PageToken, err)

}
offset = int(token.Offset)
}
Expand Down
Loading

0 comments on commit 2c11bc2

Please sign in to comment.