From 94b3862655beda86f7f4bb1ffcf2b3c0a26ba6bc Mon Sep 17 00:00:00 2001 From: Jonathan Giannuzzi Date: Thu, 6 Jul 2023 09:25:41 +0100 Subject: [PATCH 1/2] Various tooling improvements (#152) * Build binary in Makefile with the right flags * Fix Python integration tests in Makefile * Fix gitignore for Python integration tests * Refactor how integration tests are run - remove extra Dockerfiles and use the existing one instead - move docker-compose.yml to the root of the repository - cache building of the Docker image - do not clean the services when stopping them, but instead in a separate make target - run integration tests as a mounted volume instead of copying them into the container - introduce helpers to get the environment variables for the integration tests and provide defaults for them - add BuildX to CI * Add Docker in Docker to devcontainer * Rename source dir to fasttrackml within devcontainer * Default to debug log level in devcontainer --- .devcontainer/.env | 3 +- .devcontainer/devcontainer.json | 7 ++- .devcontainer/docker-compose.yml | 2 +- .github/workflows/ci.yml | 6 +++ .gitignore | 2 +- .vscode/launch.json | 10 +--- Makefile | 52 ++++++++++--------- docker-compose.yml | 33 ++++++++++++ docker/Dockerfile | 28 ---------- docker/docker-compose.yml | 38 -------------- docker/tests/integration/Dockerfile | 8 --- .../golang/aim/metric/search_test.go | 9 ++-- .../golang/aim/run/archive_batch_test.go | 8 ++- .../golang/aim/run/delete_batch_test.go | 8 ++- .../integration/golang/aim/run/delete_test.go | 7 ++- tests/integration/golang/helpers/env.go | 19 +++++++ .../golang/mlflow/experiment/create_test.go | 8 +-- .../golang/mlflow/experiment/delete_test.go | 9 ++-- .../mlflow/experiment/get_by_name_test.go | 7 ++- .../golang/mlflow/experiment/get_test.go | 7 ++- .../golang/mlflow/experiment/restore_test.go | 7 ++- .../golang/mlflow/experiment/search_test.go | 8 ++- .../experiment/set_experiment_tag_test.go | 13 +++-- .../golang/mlflow/experiment/update_test.go | 8 +-- .../golang/mlflow/run/delete_test.go | 8 ++- .../golang/mlflow/run/log_batch_test.go | 11 ++-- .../golang/mlflow/run/log_parameter_test.go | 7 ++- 27 files changed, 150 insertions(+), 183 deletions(-) create mode 100644 docker-compose.yml delete mode 100644 docker/Dockerfile delete mode 100644 docker/docker-compose.yml delete mode 100644 docker/tests/integration/Dockerfile create mode 100644 tests/integration/golang/helpers/env.go diff --git a/.devcontainer/.env b/.devcontainer/.env index 87aeb1332..c16177895 100644 --- a/.devcontainer/.env +++ b/.devcontainer/.env @@ -2,4 +2,5 @@ POSTGRES_USER=postgres POSTGRES_PASSWORD=postgres POSTGRES_DB=postgres POSTGRES_HOSTNAME=localhost -LC_COLLATE=POSIX \ No newline at end of file +LC_COLLATE=POSIX +FML_LOG_LEVEL=DEBUG \ No newline at end of file diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index 96009ba9b..84bfc819f 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -4,7 +4,7 @@ "name": "Fasttrack", "dockerComposeFile": "docker-compose.yml", "service": "app", - "workspaceFolder": "/workspaces/fasttrack", + "workspaceFolder": "/workspaces/fasttrackml", // Configure tool-specific properties. "customizations": { // Configure properties specific to VS Code. @@ -15,7 +15,7 @@ "go.useLanguageServer": true, "go.gopath": "/go", "go.goroot": "/usr/local/go", - "terminal.integrated.defaultProfile.linux": "zsh", + "terminal.integrated.defaultProfile.linux": "zsh" }, // Add the IDs of extensions you want installed when the container is created. "extensions": [ @@ -36,6 +36,9 @@ // Comment out to connect as root instead. More info: https://aka.ms/vscode-remote/containers/non-root. "remoteUser": "vscode", "features": { + "ghcr.io/devcontainers/features/docker-in-docker:2": { + "dockerDashComposeVersion": "v2" + }, "ghcr.io/devcontainers/features/git:1": {}, "ghcr.io/devcontainers/features/github-cli:1": {}, "ghcr.io/dhoeric/features/k6:1": {} diff --git a/.devcontainer/docker-compose.yml b/.devcontainer/docker-compose.yml index 3623b358a..031426187 100644 --- a/.devcontainer/docker-compose.yml +++ b/.devcontainer/docker-compose.yml @@ -31,7 +31,7 @@ services: volumes: - home:/home/vscode - workspaces:/workspaces - - ..:/workspaces/fasttrack:cached + - ..:/workspaces/fasttrackml:cached # Overrides default command so things don't shut down after the process ends. command: sleep infinity diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ba7074ddb..fa4bc1521 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -69,12 +69,16 @@ jobs: run: make test-go-unit golang-integration-tests: + if: github.event_name == 'schedule' || github.event_name == 'push' || github.event.pull_request.head.repo.id != github.event.pull_request.base.repo.id name: Golang Integration Tests runs-on: ubuntu-latest steps: - name: Checkout uses: actions/checkout@v3 + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v2 + - name: Setup Go uses: actions/setup-go@v4 with: @@ -82,6 +86,8 @@ jobs: - name: Run Integration Tests run: make service-test + env: + DOCKER_BUILDKIT: 1 python-integration-tests: if: github.event_name == 'schedule' || github.event_name == 'push' || github.event.pull_request.head.repo.id != github.event.pull_request.base.repo.id diff --git a/.gitignore b/.gitignore index 99c135bd4..dedee1f66 100644 --- a/.gitignore +++ b/.gitignore @@ -9,7 +9,7 @@ fasttrackml fasttrackml.db* # Imported integration tests -tests/*/*.src +tests/integration/python/*/*.src # Quick start outputs diff --git a/.vscode/launch.json b/.vscode/launch.json index 1ce4d3a0b..bac0578ee 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -12,10 +12,8 @@ "program": "${workspaceFolder}", "args": [ "server", - "--log-level", - "debug", "--database-uri", - "postgres://postgres:postgres@localhost:5432/postgres" + "postgres://postgres:postgres@localhost/postgres" ] }, { @@ -26,8 +24,6 @@ "program": "${workspaceFolder}", "args": [ "server", - "--log-level", - "debug", "--database-uri", "sqlite://fasttrackml.db?mode=memory&cache=shared" ] @@ -40,8 +36,6 @@ "program": "${workspaceFolder}", "args": [ "server", - "--log-level", - "debug", "--database-uri", "sqlite://fasttrackml.db" ] @@ -54,8 +48,6 @@ "program": "${workspaceFolder}", "args": [ "server", - "--log-level", - "debug", "--database-uri", "sqlite://encrypted.db?_key=passphrase" ] diff --git a/Makefile b/Makefile index 64ee9d6ea..0c9996a82 100644 --- a/Makefile +++ b/Makefile @@ -4,21 +4,12 @@ # # Project-specific variables # -# Service name. Used for binary name, docker-compose service name, etc... -SERVICE=fasttrack-service +# App name. +APP=fml # Enable Go Modules. GO111MODULE=on # -# General variables -# -# Path to Docker file -PATH_DOCKER_FILE=$(realpath ./docker/Dockerfile) -# Path to docker-compose file -PATH_DOCKER_COMPOSE_FILE=$(realpath ./docker/docker-compose.yml) -# Docker compose starting options. -DOCKER_COMPOSE_OPTIONS= -f $(PATH_DOCKER_COMPOSE_FILE) - # Default target (help) # .PHONY: help @@ -43,9 +34,9 @@ go-get: ## get go modules. @go mod download .PHONY: go-build -go-build: ## build service binary. +go-build: ## build app binary. @echo '>>> Building go binary.' - @go build -ldflags="-s -w" -o $(SERVICE) ./main.go + @go build -ldflags="-linkmode external -extldflags -static -s -w" -tags "$$(jq -r '."go.buildTags"' .vscode/settings.json)" -o $(APP) ./main.go # # Tests targets. @@ -61,23 +52,31 @@ test-go-integration: ## run go integration tests. go test -v -p 1 -tags="integration" ./tests/integration/golang/... PHONY: test-python-integration -test-python-integration: build ## run the MLFlow python integration tests. +test-python-integration: test-python-integration-mlflow test-python-integration-aim ## run all the python integration tests. + +PHONY: test-python-integration-mlflow +test-python-integration-mlflow: build ## run the MLFlow python integration tests. @echo ">>> Running MLFlow python integration tests." - tests/mlflow/test.sh + tests/integration/python/mlflow/test.sh + +PHONY: test-python-integration-aim +test-python-integration-aim: build ## run the Aim python integration tests. + @echo ">>> Running Aim python integration tests." + tests/integration/python/aim/test.sh # # Service test targets # .PHONY: service-build -service-build: ## build service and all it's dependencies - @docker-compose $(DOCKER_COMPOSE_OPTIONS) build --no-cache +service-build: ## build service and all its dependencies + @docker-compose build .PHONY: start-service-dependencies service-start-dependencies: ## start service dependencies in docker. @echo ">>> Start all Service dependencies." - @docker-compose $(DOCKER_COMPOSE_OPTIONS) up \ + @docker-compose up \ -d \ - fasttrack-postgres + postgres .PHONY: service-start service-start: service-build service-start-dependencies ## start service in docker. @@ -85,12 +84,12 @@ service-start: service-build service-start-dependencies ## start service in dock @sleep 5 @echo ">>> Starting service." @echo ">>> Starting up service container." - @docker-compose $(DOCKER_COMPOSE_OPTIONS) up -d $(SERVICE) + @docker-compose up -d service .PHONY: service-stop service-stop: ## stop service in docker. @echo ">>> Stopping service." - @docker-compose $(DOCKER_COMPOSE_OPTIONS) down -v --remove-orphans + @docker-compose stop .PHONY: service-restart service-restart: service-stop service-start ## restart service in docker @@ -98,8 +97,13 @@ service-restart: service-stop service-start ## restart service in docker .PHONY: service-test service-test: service-stop service-start ## run tests over the service in docker. @echo ">>> Running tests over service." - @docker-compose $(DOCKER_COMPOSE_OPTIONS) \ - run fasttrack-integration-tests + @docker-compose \ + run integration-tests + +.PHONY: service-clean +service-clean: ## clean service in docker. + @echo ">>> Cleaning service." + @docker-compose down -v --remove-orphans # # Mockery targets. @@ -126,4 +130,4 @@ build: go-build ## build the go components PHONY: run run: build ## run the FastTrackML server @echo ">>> Running the FasttrackML server." - ./$(SERVICE) server + ./$(APP) server diff --git a/docker-compose.yml b/docker-compose.yml new file mode 100644 index 000000000..e14a47366 --- /dev/null +++ b/docker-compose.yml @@ -0,0 +1,33 @@ +services: + service: + build: + context: . + args: + tags: netgo osusergo sqlite_foreign_keys sqlite_math_functions sqlite_omit_load_extension sqlite_unlock_notify sqlite_vacuum_incr + depends_on: + - postgres + environment: + FML_DATABASE_URI: postgres://postgres:postgres@postgres/postgres + FML_LOG_LEVEL: debug + + postgres: + image: postgres:latest + environment: + - POSTGRES_PASSWORD=postgres + + integration-tests: + image: golang:1.20 + command: make test-go-integration + volumes: + - .:/go/src + - go-cache:/go/pkg + working_dir: /go/src + depends_on: + - service + - postgres + environment: + FML_DATABASE_URI: postgres://postgres:postgres@postgres/postgres + FML_SERVICE_URI: http://service:5000 + +volumes: + go-cache: \ No newline at end of file diff --git a/docker/Dockerfile b/docker/Dockerfile deleted file mode 100644 index ae14ea94e..000000000 --- a/docker/Dockerfile +++ /dev/null @@ -1,28 +0,0 @@ -# -# Build arguments. -# -ARG GOSUMDB=off -ARG PATH_GO_SOURCES=/go/src - -# -# Build Go binary inside base container. -# -FROM golang:1.20 -# Stage arguments -ARG PATH_GO_SOURCES -# Env variables. -ENV GOSUMDB=$GOSUMDB - -# Create sources directory inside the container and copy project files. -RUN mkdir -p $PATH_GO_SOURCES/ -WORKDIR $PATH_GO_SOURCES -COPY . $PATH_GO_SOURCES - -# Build -RUN make go-get -RUN make go-build - -# Container settings. -ENV LISTEN-ADDRESS 8080 -EXPOSE 8080 -ENTRYPOINT ["./fasttrack-service", "server", "--database-uri=postgres://postgres:postgres@fasttrack-postgres:5432/postgres", "--listen-address=:8080", "--log-level=debug"] diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml deleted file mode 100644 index 28fe88dbc..000000000 --- a/docker/docker-compose.yml +++ /dev/null @@ -1,38 +0,0 @@ -version: "3.9" -services: - fasttrack-service: - build: - context: .. - dockerfile: ./docker/Dockerfile - depends_on: - - fasttrack-postgres - ports: - - "8080:8080" - networks: - - fasttrack-network - - fasttrack-postgres: - image: postgres:latest - ports: - - "5432:5432" - environment: - - POSTGRES_DB=postgres - - POSTGRES_USER=postgres - - POSTGRES_PASSWORD=postgres - networks: - - fasttrack-network - - fasttrack-integration-tests: - build: - context: ../ - dockerfile: docker/tests/integration/Dockerfile - external_links: - - fasttrack-service:fasttrack-service - environment: - DATABASE_DSN: postgres://postgres:postgres@fasttrack-postgres:5432/postgres - SERVICE_BASE_URL: http://fasttrack-service:8080 - networks: - - fasttrack-network - -networks: - fasttrack-network: diff --git a/docker/tests/integration/Dockerfile b/docker/tests/integration/Dockerfile deleted file mode 100644 index b51eba317..000000000 --- a/docker/tests/integration/Dockerfile +++ /dev/null @@ -1,8 +0,0 @@ -FROM golang:1.20 - -WORKDIR /go/src -COPY . /go/src - -RUN make go-get - -CMD ["make", "test-go-integration"] diff --git a/tests/integration/golang/aim/metric/search_test.go b/tests/integration/golang/aim/metric/search_test.go index 15e522b71..053e0404e 100644 --- a/tests/integration/golang/aim/metric/search_test.go +++ b/tests/integration/golang/aim/metric/search_test.go @@ -7,7 +7,6 @@ import ( "context" "fmt" "net/http" - "os" "testing" "github.com/google/uuid" @@ -32,12 +31,12 @@ func TestSearchMetricsTestSuite(t *testing.T) { } func (s *SearchMetricsTestSuite) SetupTest() { - s.client = helpers.NewAimApiClient(os.Getenv("SERVICE_BASE_URL")) - runFixtures, err := fixtures.NewRunFixtures(os.Getenv("DATABASE_DSN")) + s.client = helpers.NewAimApiClient(helpers.GetServiceUri()) + runFixtures, err := fixtures.NewRunFixtures(helpers.GetDatabaseUri()) assert.Nil(s.T(), err) - metricFixtures, err := fixtures.NewMetricFixtures(os.Getenv("DATABASE_DSN")) + metricFixtures, err := fixtures.NewMetricFixtures(helpers.GetDatabaseUri()) assert.Nil(s.T(), err) - experimentFixtures, err := fixtures.NewExperimentFixtures(os.Getenv("DATABASE_DSN")) + experimentFixtures, err := fixtures.NewExperimentFixtures(helpers.GetDatabaseUri()) assert.Nil(s.T(), err) s.experimentFixtures = experimentFixtures diff --git a/tests/integration/golang/aim/run/archive_batch_test.go b/tests/integration/golang/aim/run/archive_batch_test.go index 3c62909d4..5e2a5704a 100644 --- a/tests/integration/golang/aim/run/archive_batch_test.go +++ b/tests/integration/golang/aim/run/archive_batch_test.go @@ -5,7 +5,6 @@ package run import ( "context" "fmt" - "os" "testing" "github.com/gofiber/fiber/v2" @@ -31,11 +30,11 @@ func TestArchiveBatchTestSuite(t *testing.T) { } func (s *ArchiveBatchTestSuite) SetupTest() { - s.client = helpers.NewAimApiClient(os.Getenv("SERVICE_BASE_URL")) - runFixtures, err := fixtures.NewRunFixtures(os.Getenv("DATABASE_DSN")) + s.client = helpers.NewAimApiClient(helpers.GetServiceUri()) + runFixtures, err := fixtures.NewRunFixtures(helpers.GetDatabaseUri()) assert.Nil(s.T(), err) s.runFixtures = runFixtures - expFixtures, err := fixtures.NewExperimentFixtures(os.Getenv("DATABASE_DSN")) + expFixtures, err := fixtures.NewExperimentFixtures(helpers.GetDatabaseUri()) assert.Nil(s.T(), err) s.experimentFixtures = expFixtures @@ -109,7 +108,6 @@ func (s *ArchiveBatchTestSuite) Test_Ok() { assert.Nil(s.T(), err) assert.Equal(s.T(), originalMinRowNum, newMinRowNum) assert.Equal(s.T(), originalMaxRowNum, newMaxRowNum) - }) } } diff --git a/tests/integration/golang/aim/run/delete_batch_test.go b/tests/integration/golang/aim/run/delete_batch_test.go index c48b65238..ca2ed670b 100644 --- a/tests/integration/golang/aim/run/delete_batch_test.go +++ b/tests/integration/golang/aim/run/delete_batch_test.go @@ -4,7 +4,6 @@ package run import ( "context" - "os" "testing" "github.com/gofiber/fiber/v2" @@ -31,11 +30,11 @@ func TestDeleteBatchTestSuite(t *testing.T) { } func (s *DeleteBatchTestSuite) SetupTest() { - s.client = helpers.NewAimApiClient(os.Getenv("SERVICE_BASE_URL")) - runFixtures, err := fixtures.NewRunFixtures(os.Getenv("DATABASE_DSN")) + s.client = helpers.NewAimApiClient(helpers.GetServiceUri()) + runFixtures, err := fixtures.NewRunFixtures(helpers.GetDatabaseUri()) assert.Nil(s.T(), err) s.runFixtures = runFixtures - expFixtures, err := fixtures.NewExperimentFixtures(os.Getenv("DATABASE_DSN")) + expFixtures, err := fixtures.NewExperimentFixtures(helpers.GetDatabaseUri()) assert.Nil(s.T(), err) s.experimentFixtures = expFixtures @@ -93,7 +92,6 @@ func (s *DeleteBatchTestSuite) Test_Ok() { assert.Nil(s.T(), err) assert.Equal(s.T(), originalMinRowNum, newMinRowNum) assert.Greater(s.T(), originalMaxRowNum, newMaxRowNum) - }) } } diff --git a/tests/integration/golang/aim/run/delete_test.go b/tests/integration/golang/aim/run/delete_test.go index 360446218..ad96b5c02 100644 --- a/tests/integration/golang/aim/run/delete_test.go +++ b/tests/integration/golang/aim/run/delete_test.go @@ -5,7 +5,6 @@ package run import ( "context" "fmt" - "os" "testing" "github.com/gofiber/fiber/v2" @@ -34,13 +33,13 @@ func TestDeleteRunTestSuite(t *testing.T) { } func (s *DeleteRunTestSuite) SetupTest() { - s.client = helpers.NewAimApiClient(os.Getenv("SERVICE_BASE_URL")) + s.client = helpers.NewAimApiClient(helpers.GetServiceUri()) - runFixtures, err := fixtures.NewRunFixtures(os.Getenv("DATABASE_DSN")) + runFixtures, err := fixtures.NewRunFixtures(helpers.GetDatabaseUri()) assert.Nil(s.T(), err) s.runFixtures = runFixtures - expFixtures, err := fixtures.NewExperimentFixtures(os.Getenv("DATABASE_DSN")) + expFixtures, err := fixtures.NewExperimentFixtures(helpers.GetDatabaseUri()) assert.Nil(s.T(), err) s.experimentFixtures = expFixtures diff --git a/tests/integration/golang/helpers/env.go b/tests/integration/golang/helpers/env.go new file mode 100644 index 000000000..08d4fcb3b --- /dev/null +++ b/tests/integration/golang/helpers/env.go @@ -0,0 +1,19 @@ +package helpers + +import "os" + +func GetDatabaseUri() string { + uri, ok := os.LookupEnv("FML_DATABASE_URI") + if ok { + return uri + } + return "sqlite://fasttrackml.db" +} + +func GetServiceUri() string { + uri, ok := os.LookupEnv("FML_SERVICE_URI") + if ok { + return uri + } + return "http://localhost:5000" +} diff --git a/tests/integration/golang/mlflow/experiment/create_test.go b/tests/integration/golang/mlflow/experiment/create_test.go index 200fc664d..e16e8c92b 100644 --- a/tests/integration/golang/mlflow/experiment/create_test.go +++ b/tests/integration/golang/mlflow/experiment/create_test.go @@ -4,7 +4,6 @@ package experiment import ( "fmt" - "os" "testing" "github.com/stretchr/testify/assert" @@ -29,8 +28,8 @@ func TestCreateExperimentTestSuite(t *testing.T) { } func (s *CreateExperimentTestSuite) SetupTest() { - s.client = helpers.NewMlflowApiClient(os.Getenv("SERVICE_BASE_URL")) - fixtures, err := fixtures.NewExperimentFixtures(os.Getenv("DATABASE_DSN")) + s.client = helpers.NewMlflowApiClient(helpers.GetServiceUri()) + fixtures, err := fixtures.NewExperimentFixtures(helpers.GetDatabaseUri()) assert.Nil(s.T(), err) s.fixtures = fixtures } @@ -63,8 +62,9 @@ func (s *CreateExperimentTestSuite) Test_Ok() { assert.Nil(s.T(), err) assert.NotEmpty(s.T(), resp.ID) } + func (s *CreateExperimentTestSuite) Test_Error() { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.CreateExperimentRequest diff --git a/tests/integration/golang/mlflow/experiment/delete_test.go b/tests/integration/golang/mlflow/experiment/delete_test.go index 8f66d824d..55aea3669 100644 --- a/tests/integration/golang/mlflow/experiment/delete_test.go +++ b/tests/integration/golang/mlflow/experiment/delete_test.go @@ -6,7 +6,6 @@ import ( "context" "database/sql" "fmt" - "os" "testing" "time" @@ -33,8 +32,8 @@ func TestDeleteExperimentTestSuite(t *testing.T) { } func (s *DeleteExperimentTestSuite) SetupTest() { - s.client = helpers.NewMlflowApiClient(os.Getenv("SERVICE_BASE_URL")) - fixtures, err := fixtures.NewExperimentFixtures(os.Getenv("DATABASE_DSN")) + s.client = helpers.NewMlflowApiClient(helpers.GetServiceUri()) + fixtures, err := fixtures.NewExperimentFixtures(helpers.GetDatabaseUri()) assert.Nil(s.T(), err) s.fixtures = fixtures } @@ -65,7 +64,7 @@ func (s *DeleteExperimentTestSuite) Test_Ok() { assert.Nil(s.T(), s.fixtures.UnloadFixtures()) }() - //check that the experiment lifecycle is active + // check that the experiment lifecycle is active assert.Equal(s.T(), models.LifecycleStageActive, experiment.LifecycleStage) // 2. make actual API call. @@ -87,7 +86,7 @@ func (s *DeleteExperimentTestSuite) Test_Ok() { } func (s *DeleteExperimentTestSuite) Test_Error() { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.DeleteExperimentRequest diff --git a/tests/integration/golang/mlflow/experiment/get_by_name_test.go b/tests/integration/golang/mlflow/experiment/get_by_name_test.go index cdf350077..baf6f4356 100644 --- a/tests/integration/golang/mlflow/experiment/get_by_name_test.go +++ b/tests/integration/golang/mlflow/experiment/get_by_name_test.go @@ -6,7 +6,6 @@ import ( "context" "database/sql" "fmt" - "os" "testing" "time" @@ -34,8 +33,8 @@ func TestGetExperimentByNameTestSuite(t *testing.T) { } func (s *GetExperimentByNameTestSuite) SetupTest() { - s.client = helpers.NewMlflowApiClient(os.Getenv("SERVICE_BASE_URL")) - fixtures, err := fixtures.NewExperimentFixtures(os.Getenv("DATABASE_DSN")) + s.client = helpers.NewMlflowApiClient(helpers.GetServiceUri()) + fixtures, err := fixtures.NewExperimentFixtures(helpers.GetDatabaseUri()) assert.Nil(s.T(), err) s.fixtures = fixtures } @@ -95,7 +94,7 @@ func (s *GetExperimentByNameTestSuite) Test_Ok() { } func (s *GetExperimentByNameTestSuite) Test_Error() { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.GetExperimentRequest diff --git a/tests/integration/golang/mlflow/experiment/get_test.go b/tests/integration/golang/mlflow/experiment/get_test.go index ec4bf3c19..8e43cdfa6 100644 --- a/tests/integration/golang/mlflow/experiment/get_test.go +++ b/tests/integration/golang/mlflow/experiment/get_test.go @@ -6,7 +6,6 @@ import ( "context" "database/sql" "fmt" - "os" "testing" "time" @@ -34,8 +33,8 @@ func TestGetExperimentTestSuite(t *testing.T) { } func (s *GetExperimentTestSuite) SetupTest() { - s.client = helpers.NewMlflowApiClient(os.Getenv("SERVICE_BASE_URL")) - fixtures, err := fixtures.NewExperimentFixtures(os.Getenv("DATABASE_DSN")) + s.client = helpers.NewMlflowApiClient(helpers.GetServiceUri()) + fixtures, err := fixtures.NewExperimentFixtures(helpers.GetDatabaseUri()) assert.Nil(s.T(), err) s.fixtures = fixtures } @@ -95,7 +94,7 @@ func (s *GetExperimentTestSuite) Test_Ok() { } func (s *GetExperimentTestSuite) Test_Error() { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.GetExperimentRequest diff --git a/tests/integration/golang/mlflow/experiment/restore_test.go b/tests/integration/golang/mlflow/experiment/restore_test.go index cb78d2dc1..d2f9a6333 100644 --- a/tests/integration/golang/mlflow/experiment/restore_test.go +++ b/tests/integration/golang/mlflow/experiment/restore_test.go @@ -6,7 +6,6 @@ import ( "context" "database/sql" "fmt" - "os" "testing" "time" @@ -34,8 +33,8 @@ func TestRestoreExperimentTestSuite(t *testing.T) { } func (s *RestoreExperimentTestSuite) SetupTest() { - s.client = helpers.NewMlflowApiClient(os.Getenv("SERVICE_BASE_URL")) - fixtures, err := fixtures.NewExperimentFixtures(os.Getenv("DATABASE_DSN")) + s.client = helpers.NewMlflowApiClient(helpers.GetServiceUri()) + fixtures, err := fixtures.NewExperimentFixtures(helpers.GetDatabaseUri()) assert.Nil(s.T(), err) s.fixtures = fixtures } @@ -87,7 +86,7 @@ func (s *RestoreExperimentTestSuite) Test_Ok() { } func (s *RestoreExperimentTestSuite) Test_Error() { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.RestoreExperimentRequest diff --git a/tests/integration/golang/mlflow/experiment/search_test.go b/tests/integration/golang/mlflow/experiment/search_test.go index bf104a476..1bef9ba5b 100644 --- a/tests/integration/golang/mlflow/experiment/search_test.go +++ b/tests/integration/golang/mlflow/experiment/search_test.go @@ -6,7 +6,6 @@ import ( "context" "database/sql" "fmt" - "os" "testing" "time" @@ -34,8 +33,8 @@ func TestSearchExperimentsTestSuite(t *testing.T) { } func (s *SearchExperimentsTestSuite) SetupTest() { - s.client = helpers.NewMlflowApiClient(os.Getenv("SERVICE_BASE_URL")) - fixtures, err := fixtures.NewExperimentFixtures(os.Getenv("DATABASE_DSN")) + s.client = helpers.NewMlflowApiClient(helpers.GetServiceUri()) + fixtures, err := fixtures.NewExperimentFixtures(helpers.GetDatabaseUri()) assert.Nil(s.T(), err) s.fixtures = fixtures } @@ -175,7 +174,7 @@ func (s *SearchExperimentsTestSuite) Test_Ok() { } func (s *SearchExperimentsTestSuite) Test_Error() { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.SearchExperimentsRequest @@ -258,5 +257,4 @@ func (s *SearchExperimentsTestSuite) Test_Error() { assert.Equal(s.T(), tt.error.Error(), resp.Error()) }) } - } diff --git a/tests/integration/golang/mlflow/experiment/set_experiment_tag_test.go b/tests/integration/golang/mlflow/experiment/set_experiment_tag_test.go index fa9c39851..13590e32d 100644 --- a/tests/integration/golang/mlflow/experiment/set_experiment_tag_test.go +++ b/tests/integration/golang/mlflow/experiment/set_experiment_tag_test.go @@ -6,7 +6,6 @@ import ( "context" "database/sql" "fmt" - "os" "testing" "time" @@ -32,8 +31,8 @@ func TestSetExperimentTagTestSuite(t *testing.T) { } func (s *SetExperimentTagTestSuite) SetupTest() { - s.client = helpers.NewMlflowApiClient(os.Getenv("SERVICE_BASE_URL")) - fixtures, err := fixtures.NewExperimentFixtures(os.Getenv("DATABASE_DSN")) + s.client = helpers.NewMlflowApiClient(helpers.GetServiceUri()) + fixtures, err := fixtures.NewExperimentFixtures(helpers.GetDatabaseUri()) assert.Nil(s.T(), err) s.fixtures = fixtures } @@ -102,12 +101,12 @@ func (s *SetExperimentTagTestSuite) Test_Ok() { assert.True(s.T(), helpers.CheckTagExists(exp.Tags, "KeyTag1", "ValueTag2"), "Expected 'experiment.tags' to contain 'KeyTag1' with value 'ValueTag1'") - //test that setting a tag on 1 experiment does not impact another experiment. + // test that setting a tag on 1 experiment does not impact another experiment. exp, err = s.fixtures.GetExperimentByID(context.Background(), *experiment1.ID) assert.Nil(s.T(), err) assert.Equal(s.T(), len(exp.Tags), 0) - //test that setting a tag on different experiments maintain different values across experiments + // test that setting a tag on different experiments maintain different values across experiments SetExperimentTag(s, experiment1, "KeyTag1", "ValueTag3") exp, err = s.fixtures.GetExperimentByID(context.Background(), *experiment.ID) assert.Nil(s.T(), err) @@ -115,10 +114,10 @@ func (s *SetExperimentTagTestSuite) Test_Ok() { assert.Nil(s.T(), err) assert.True(s.T(), helpers.CheckTagExists(exp.Tags, "KeyTag1", "ValueTag2"), "Expected 'experiment.tags' to contain 'KeyTag1' with value 'ValueTag2'") assert.True(s.T(), helpers.CheckTagExists(exp1.Tags, "KeyTag1", "ValueTag3"), "Expected 'experiment.tags' to contain 'KeyTag1' with value 'ValueTag3'") - } + func (s *SetExperimentTagTestSuite) Test_Error() { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.SetExperimentTagRequest diff --git a/tests/integration/golang/mlflow/experiment/update_test.go b/tests/integration/golang/mlflow/experiment/update_test.go index a9d45296c..8ce85e490 100644 --- a/tests/integration/golang/mlflow/experiment/update_test.go +++ b/tests/integration/golang/mlflow/experiment/update_test.go @@ -6,7 +6,6 @@ import ( "context" "database/sql" "fmt" - "os" "testing" "time" @@ -32,8 +31,8 @@ func TestUpdateExperimentTestSuite(t *testing.T) { } func (s *UpdateExperimentTestSuite) SetupTest() { - s.client = helpers.NewMlflowApiClient(os.Getenv("SERVICE_BASE_URL")) - fixtures, err := fixtures.NewExperimentFixtures(os.Getenv("DATABASE_DSN")) + s.client = helpers.NewMlflowApiClient(helpers.GetServiceUri()) + fixtures, err := fixtures.NewExperimentFixtures(helpers.GetDatabaseUri()) assert.Nil(s.T(), err) s.fixtures = fixtures } @@ -73,8 +72,9 @@ func (s *UpdateExperimentTestSuite) Test_Ok() { assert.Nil(s.T(), err) assert.Equal(s.T(), "Test Updated Experiment", exp.Name) } + func (s *UpdateExperimentTestSuite) Test_Error() { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.UpdateExperimentRequest diff --git a/tests/integration/golang/mlflow/run/delete_test.go b/tests/integration/golang/mlflow/run/delete_test.go index 67cf26d76..4e61cc723 100644 --- a/tests/integration/golang/mlflow/run/delete_test.go +++ b/tests/integration/golang/mlflow/run/delete_test.go @@ -5,7 +5,6 @@ package run import ( "context" "fmt" - "os" "testing" "github.com/google/uuid" @@ -33,11 +32,11 @@ func TestDeleteRunTestSuite(t *testing.T) { } func (s *DeleteRunTestSuite) SetupTest() { - s.client = helpers.NewMlflowApiClient(os.Getenv("SERVICE_BASE_URL")) - runFixtures, err := fixtures.NewRunFixtures(os.Getenv("DATABASE_DSN")) + s.client = helpers.NewMlflowApiClient(helpers.GetServiceUri()) + runFixtures, err := fixtures.NewRunFixtures(helpers.GetDatabaseUri()) assert.Nil(s.T(), err) s.runFixtures = runFixtures - expFixtures, err := fixtures.NewExperimentFixtures(os.Getenv("DATABASE_DSN")) + expFixtures, err := fixtures.NewExperimentFixtures(helpers.GetDatabaseUri()) assert.Nil(s.T(), err) s.experimentFixtures = expFixtures @@ -113,7 +112,6 @@ func (s *DeleteRunTestSuite) Test_Error() { ) assert.Nil(s.T(), err) assert.Equal(s.T(), "RESOURCE_DOES_NOT_EXIST: unable to find run 'not-an-id': error getting `run` entity by id: not-an-id: record not found", resp.Error()) - }) } } diff --git a/tests/integration/golang/mlflow/run/log_batch_test.go b/tests/integration/golang/mlflow/run/log_batch_test.go index 1b05a073d..c5b1e3236 100644 --- a/tests/integration/golang/mlflow/run/log_batch_test.go +++ b/tests/integration/golang/mlflow/run/log_batch_test.go @@ -5,7 +5,6 @@ package run import ( "context" "fmt" - "os" "strings" "testing" @@ -35,14 +34,14 @@ func TestLogBatchTestSuite(t *testing.T) { } func (s *LogBatchTestSuite) SetupTest() { - s.client = helpers.NewMlflowApiClient(os.Getenv("SERVICE_BASE_URL")) - runFixtures, err := fixtures.NewRunFixtures(os.Getenv("DATABASE_DSN")) + s.client = helpers.NewMlflowApiClient(helpers.GetServiceUri()) + runFixtures, err := fixtures.NewRunFixtures(helpers.GetDatabaseUri()) assert.Nil(s.T(), err) s.runFixtures = runFixtures - metricFixtures, err := fixtures.NewMetricFixtures(os.Getenv("DATABASE_DSN")) + metricFixtures, err := fixtures.NewMetricFixtures(helpers.GetDatabaseUri()) assert.Nil(s.T(), err) s.metricFixtures = metricFixtures - expFixtures, err := fixtures.NewExperimentFixtures(os.Getenv("DATABASE_DSN")) + expFixtures, err := fixtures.NewExperimentFixtures(helpers.GetDatabaseUri()) assert.Nil(s.T(), err) s.experimentFixtures = expFixtures @@ -267,7 +266,7 @@ func (s *LogBatchTestSuite) Test_Error() { assert.Nil(s.T(), s.runFixtures.UnloadFixtures()) }() - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.LogBatchRequest diff --git a/tests/integration/golang/mlflow/run/log_parameter_test.go b/tests/integration/golang/mlflow/run/log_parameter_test.go index 562aa6ff6..788566ff5 100644 --- a/tests/integration/golang/mlflow/run/log_parameter_test.go +++ b/tests/integration/golang/mlflow/run/log_parameter_test.go @@ -5,7 +5,6 @@ package run import ( "context" "fmt" - "os" "strings" "testing" @@ -34,11 +33,11 @@ func TestLogParamTestSuite(t *testing.T) { } func (s *LogParamTestSuite) SetupTest() { - s.client = helpers.NewMlflowApiClient(os.Getenv("SERVICE_BASE_URL")) - runFixtures, err := fixtures.NewRunFixtures(os.Getenv("DATABASE_DSN")) + s.client = helpers.NewMlflowApiClient(helpers.GetServiceUri()) + runFixtures, err := fixtures.NewRunFixtures(helpers.GetDatabaseUri()) assert.Nil(s.T(), err) s.runFixtures = runFixtures - expFixtures, err := fixtures.NewExperimentFixtures(os.Getenv("DATABASE_DSN")) + expFixtures, err := fixtures.NewExperimentFixtures(helpers.GetDatabaseUri()) assert.Nil(s.T(), err) s.experimentFixtures = expFixtures From 2c11bc2a11a0609e6f581c540a33464cfc79e5e1 Mon Sep 17 00:00:00 2001 From: Jonathan Giannuzzi Date: Thu, 6 Jul 2023 17:39:23 +0100 Subject: [PATCH 2/2] Add stricter formatters (#155) * Add stricter formatters * Apply stricter formatting --- .devcontainer/Dockerfile | 6 ++- .github/workflows/ci.yml | 21 +++++++-- .vscode/settings.json | 7 ++- Makefile | 6 +++ main.go | 4 +- pkg/api/aim/apps.go | 4 +- pkg/api/aim/dashboards.go | 5 +- pkg/api/aim/encoding/encoder.go | 6 ++- pkg/api/aim/experiments.go | 4 +- pkg/api/aim/projects.go | 4 +- pkg/api/aim/runs.go | 8 ++-- pkg/api/aim/runs_test.go | 46 +++++++++---------- pkg/api/mlflow/api/response/metric_test.go | 4 +- pkg/api/mlflow/api/response/run_test.go | 2 +- pkg/api/mlflow/controller/metric.go | 2 +- pkg/api/mlflow/dao/convertors/log_test.go | 1 - pkg/api/mlflow/dao/convertors/run.go | 3 +- pkg/api/mlflow/dao/convertors/run_test.go | 3 +- pkg/api/mlflow/dao/repositories/experiment.go | 3 -- .../service/artifact/validators_test.go | 3 +- pkg/api/mlflow/service/experiment/service.go | 1 - .../mlflow/service/experiment/service_test.go | 21 ++++++--- .../service/experiment/validators_test.go | 24 ++++++---- pkg/api/mlflow/service/metric/service_test.go | 9 ++-- .../mlflow/service/metric/validators_test.go | 9 ++-- pkg/api/mlflow/service/run/service.go | 1 - pkg/api/mlflow/service/run/service_test.go | 26 +++++++---- pkg/api/mlflow/service/run/validators_test.go | 31 ++++++++----- pkg/cmd/root.go | 4 +- tests/integration/golang/fixtures/run.go | 6 ++- 30 files changed, 167 insertions(+), 107 deletions(-) diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index cf079eaaa..dedba931e 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -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. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fa4bc1521..63c713371 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -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 @@ -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 diff --git a/.vscode/settings.json b/.vscode/settings.json index e79d0f5a2..33783062a 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -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 + } } \ No newline at end of file diff --git a/Makefile b/Makefile index 0c9996a82..25b8a730e 100644 --- a/Makefile +++ b/Makefile @@ -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. # diff --git a/main.go b/main.go index d1d0b1dc0..51abd14ef 100644 --- a/main.go +++ b/main.go @@ -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() { diff --git a/pkg/api/aim/apps.go b/pkg/api/aim/apps.go index 1fe402f10..b29fcea87 100644 --- a/pkg/api/aim/apps.go +++ b/pkg/api/aim/apps.go @@ -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 { diff --git a/pkg/api/aim/dashboards.go b/pkg/api/aim/dashboards.go index 253acd6f7..4a5b3b712 100644 --- a/pkg/api/aim/dashboards.go +++ b/pkg/api/aim/dashboards.go @@ -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 { diff --git a/pkg/api/aim/encoding/encoder.go b/pkg/api/aim/encoding/encoder.go index c35f0bf74..e1a0030f6 100644 --- a/pkg/api/aim/encoding/encoder.go +++ b/pkg/api/aim/encoding/encoder.go @@ -8,8 +8,10 @@ import ( "reflect" ) -type arrayFlag struct{} -type objectFlag struct{} +type ( + arrayFlag struct{} + objectFlag struct{} +) var pathSentinel = []byte{0xfe} diff --git a/pkg/api/aim/experiments.go b/pkg/api/aim/experiments.go index fe6a619aa..ce097e0e4 100644 --- a/pkg/api/aim/experiments.go +++ b/pkg/api/aim/experiments.go @@ -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 { diff --git a/pkg/api/aim/projects.go b/pkg/api/aim/projects.go index 42b1b71bf..03f85b266 100644 --- a/pkg/api/aim/projects.go +++ b/pkg/api/aim/projects.go @@ -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 { diff --git a/pkg/api/aim/runs.go b/pkg/api/aim/runs.go index a72bd80ab..1ee6ac78c 100644 --- a/pkg/api/aim/runs.go +++ b/pkg/api/aim/runs.go @@ -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 diff --git a/pkg/api/aim/runs_test.go b/pkg/api/aim/runs_test.go index 6e1729902..699d235f2 100644 --- a/pkg/api/aim/runs_test.go +++ b/pkg/api/aim/runs_test.go @@ -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, }, } diff --git a/pkg/api/mlflow/api/response/metric_test.go b/pkg/api/mlflow/api/response/metric_test.go index 44c725ec4..73d73168a 100644 --- a/pkg/api/mlflow/api/response/metric_test.go +++ b/pkg/api/mlflow/api/response/metric_test.go @@ -9,7 +9,7 @@ import ( ) func TestNewMetricHistoryResponse_Ok(t *testing.T) { - var testData = []struct { + testData := []struct { name string metrics []models.Metric expectedResponse *GetMetricHistoryResponse @@ -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 diff --git a/pkg/api/mlflow/api/response/run_test.go b/pkg/api/mlflow/api/response/run_test.go index 3945498d1..7b1de9c89 100644 --- a/pkg/api/mlflow/api/response/run_test.go +++ b/pkg/api/mlflow/api/response/run_test.go @@ -10,7 +10,7 @@ import ( ) func TestNewRunPartialResponse(t *testing.T) { - var testData = []struct { + testData := []struct { name string run *models.Run expectedResponse *RunPartialResponse diff --git a/pkg/api/mlflow/controller/metric.go b/pkg/api/mlflow/controller/metric.go index f237c482d..c7beb9906 100644 --- a/pkg/api/mlflow/controller/metric.go +++ b/pkg/api/mlflow/controller/metric.go @@ -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" @@ -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. diff --git a/pkg/api/mlflow/dao/convertors/log_test.go b/pkg/api/mlflow/dao/convertors/log_test.go index 2ac8256ed..9ad4cd762 100644 --- a/pkg/api/mlflow/dao/convertors/log_test.go +++ b/pkg/api/mlflow/dao/convertors/log_test.go @@ -28,7 +28,6 @@ func TestConvertLogBatchRequestToDBModel_Ok(t *testing.T) { }}, Params: []request.ParamPartialRequest{ { - Key: "key", Value: "value", }, diff --git a/pkg/api/mlflow/dao/convertors/run.go b/pkg/api/mlflow/dao/convertors/run.go index fca603346..8dcbd2fc9 100644 --- a/pkg/api/mlflow/dao/convertors/run.go +++ b/pkg/api/mlflow/dao/convertors/run.go @@ -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" diff --git a/pkg/api/mlflow/dao/convertors/run_test.go b/pkg/api/mlflow/dao/convertors/run_test.go index 8e75c36b2..f66d7def1 100644 --- a/pkg/api/mlflow/dao/convertors/run_test.go +++ b/pkg/api/mlflow/dao/convertors/run_test.go @@ -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) diff --git a/pkg/api/mlflow/dao/repositories/experiment.go b/pkg/api/mlflow/dao/repositories/experiment.go index 67f52249a..ffe3e685c 100644 --- a/pkg/api/mlflow/dao/repositories/experiment.go +++ b/pkg/api/mlflow/dao/repositories/experiment.go @@ -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) } @@ -100,7 +98,6 @@ func (r ExperimentRepository) Update(ctx context.Context, experiment *models.Exp } } return nil - }); err != nil { return err } diff --git a/pkg/api/mlflow/service/artifact/validators_test.go b/pkg/api/mlflow/service/artifact/validators_test.go index 6b15a580f..3deb7b243 100644 --- a/pkg/api/mlflow/service/artifact/validators_test.go +++ b/pkg/api/mlflow/service/artifact/validators_test.go @@ -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 diff --git a/pkg/api/mlflow/service/experiment/service.go b/pkg/api/mlflow/service/experiment/service.go index 3b9ec77c2..ea0ea8b98 100644 --- a/pkg/api/mlflow/service/experiment/service.go +++ b/pkg/api/mlflow/service/experiment/service.go @@ -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) } diff --git a/pkg/api/mlflow/service/experiment/service_test.go b/pkg/api/mlflow/service/experiment/service_test.go index 34d7e7c2e..718e13aa4 100644 --- a/pkg/api/mlflow/service/experiment/service_test.go +++ b/pkg/api/mlflow/service/experiment/service_test.go @@ -53,8 +53,9 @@ func TestService_CreateExperiment_Ok(t *testing.T) { assert.NotEmpty(t, experiment.CreationTime.Int64) assert.NotEmpty(t, experiment.LastUpdateTime.Int64) } + func TestService_CreateExperiment_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.CreateExperimentRequest @@ -207,8 +208,9 @@ func TestService_DeleteExperiment_Ok(t *testing.T) { // compare results. assert.Nil(t, err) } + func TestService_DeleteExperiment_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.DeleteExperimentRequest @@ -340,8 +342,9 @@ func TestService_GetExperiment_Ok(t *testing.T) { assert.Equal(t, sql.NullInt64{Int64: 1234567891, Valid: true}, experiment.LastUpdateTime) assert.Equal(t, "/artifact/location", experiment.ArtifactLocation) } + func TestService_GetExperiment_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.GetExperimentRequest @@ -452,8 +455,9 @@ func TestService_GetExperimentByName_Ok(t *testing.T) { assert.Equal(t, sql.NullInt64{Int64: 1234567891, Valid: true}, experiment.LastUpdateTime) assert.Equal(t, "/artifact/location", experiment.ArtifactLocation) } + func TestService_GetExperimentByName_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.GetExperimentRequest @@ -542,8 +546,9 @@ func TestService_RestoreExperiment_Ok(t *testing.T) { // compare results. assert.Nil(t, err) } + func TestService_RestoreExperiment_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.RestoreExperimentRequest @@ -655,8 +660,9 @@ func TestService_SetExperimentTag_Ok(t *testing.T) { // compare results. assert.Nil(t, err) } + func TestService_SetExperimentTag_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.SetExperimentTagRequest @@ -785,8 +791,9 @@ func TestService_UpdateExperiment_Ok(t *testing.T) { // compare results. assert.Nil(t, err) } + func TestService_UpdateExperiment_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.UpdateExperimentRequest diff --git a/pkg/api/mlflow/service/experiment/validators_test.go b/pkg/api/mlflow/service/experiment/validators_test.go index 62c3407e2..1d9fa97e3 100644 --- a/pkg/api/mlflow/service/experiment/validators_test.go +++ b/pkg/api/mlflow/service/experiment/validators_test.go @@ -16,8 +16,9 @@ func TestValidateCreateExperimentRequest_Ok(t *testing.T) { }) assert.Nil(t, err) } + func TestValidateCreateExperimentRequest_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.CreateExperimentRequest @@ -54,8 +55,9 @@ func TestValidateUpdateExperimentRequest_Ok(t *testing.T) { }) assert.Nil(t, err) } + func TestValidateUpdateExperimentRequest_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.UpdateExperimentRequest @@ -88,8 +90,9 @@ func TestValidateGetExperimentByIDRequest_Ok(t *testing.T) { }) assert.Nil(t, err) } + func TestValidateGetExperimentByIDRequest_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.UpdateExperimentRequest @@ -122,8 +125,9 @@ func TestValidateGetExperimentByNameRequest_Ok(t *testing.T) { }) assert.Nil(t, err) } + func TestValidateGetExperimentByNameRequest_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.GetExperimentRequest @@ -149,8 +153,9 @@ func TestValidateDeleteExperimentRequest_Ok(t *testing.T) { }) assert.Nil(t, err) } + func TestValidateDeleteExperimentRequest_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.DeleteExperimentRequest @@ -176,8 +181,9 @@ func TestValidateRestoreExperimentRequest_Ok(t *testing.T) { }) assert.Nil(t, err) } + func TestValidateRestoreExperimentRequest_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.RestoreExperimentRequest @@ -204,8 +210,9 @@ func TestValidateSearchExperimentsRequest_Ok(t *testing.T) { }) assert.Nil(t, err) } + func TestValidateSearchExperimentsRequest_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.SearchExperimentsRequest @@ -242,8 +249,9 @@ func TestValidateSetExperimentTagRequest_Ok(t *testing.T) { }) assert.Nil(t, err) } + func TestValidateSetExperimentTagRequest_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.SetExperimentTagRequest diff --git a/pkg/api/mlflow/service/metric/service_test.go b/pkg/api/mlflow/service/metric/service_test.go index 97f3a2e9e..a111a422b 100644 --- a/pkg/api/mlflow/service/metric/service_test.go +++ b/pkg/api/mlflow/service/metric/service_test.go @@ -50,8 +50,9 @@ func TestService_GetMetricHistory_Ok(t *testing.T) { }, }, metrics) } + func TestService_GetMetricHistory_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.GetMetricHistoryRequest @@ -143,8 +144,9 @@ func TestService_GetMetricHistoryBulk_Ok(t *testing.T) { }, }, metrics) } + func TestService_GetMetricHistoryBulk_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.GetMetricHistoryBulkRequest @@ -249,8 +251,9 @@ func TestNewService_GetMetricHistories_Ok(t *testing.T) { assert.NotNil(t, rows) assert.NotNil(t, iterator) } + func TestNewService_GetMetricHistories_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.GetMetricHistoriesRequest diff --git a/pkg/api/mlflow/service/metric/validators_test.go b/pkg/api/mlflow/service/metric/validators_test.go index 65a6fb74e..eee5f3a59 100644 --- a/pkg/api/mlflow/service/metric/validators_test.go +++ b/pkg/api/mlflow/service/metric/validators_test.go @@ -17,8 +17,9 @@ func TestValidateGetMetricHistoryRequest_Ok(t *testing.T) { }) assert.Nil(t, err) } + func TestValidateGetMetricHistoryRequest_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.GetMetricHistoryRequest @@ -53,8 +54,9 @@ func TestValidateGetMetricHistoryBulkRequest_Ok(t *testing.T) { }) assert.Nil(t, err) } + func TestValidateGetMetricHistoryBulkRequest_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.GetMetricHistoryBulkRequest @@ -98,8 +100,9 @@ func TestValidateGetMetricHistoriesRequest_Ok(t *testing.T) { }) assert.Nil(t, err) } + func TestValidateGetMetricHistoriesRequest_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.GetMetricHistoriesRequest diff --git a/pkg/api/mlflow/service/run/service.go b/pkg/api/mlflow/service/run/service.go index 6c76b2e80..771761a05 100644 --- a/pkg/api/mlflow/service/run/service.go +++ b/pkg/api/mlflow/service/run/service.go @@ -166,7 +166,6 @@ func (s Service) SearchRuns(ctx context.Context, req *request.SearchRunsRequest) ), ).Decode(&token); err != nil { return nil, 0, 0, api.NewInvalidParameterValueError("invalid page_token '%s': %s", req.PageToken, err) - } offset = int(token.Offset) } diff --git a/pkg/api/mlflow/service/run/service_test.go b/pkg/api/mlflow/service/run/service_test.go index 327006e88..cba85ca1c 100644 --- a/pkg/api/mlflow/service/run/service_test.go +++ b/pkg/api/mlflow/service/run/service_test.go @@ -89,8 +89,9 @@ func TestService_CreateRun_Ok(t *testing.T) { }, }, run.Tags) } + func TestService_CreateRun_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.CreateRunRequest @@ -197,8 +198,9 @@ func TestService_CreateRun_Error(t *testing.T) { func TestService_UpdateRun_Ok(t *testing.T) { // TODO:DSuhinin skip this test for now. I don't know how to mock `gorm` transaction logic. } + func TestService_UpdateRun_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.UpdateRunRequest @@ -282,8 +284,9 @@ func TestService_RestoreRun_Ok(t *testing.T) { // compare results. assert.Nil(t, err) } + func TestService_RestoreRun_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.RestoreRunRequest @@ -435,8 +438,9 @@ func TestService_DeleteRun_Ok(t *testing.T) { // compare results. assert.Nil(t, err) } + func TestService_DeleteRun_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.DeleteRunRequest @@ -545,7 +549,7 @@ func TestService_DeleteRun_Error(t *testing.T) { func TestService_DeleteRunTag_Ok(t *testing.T) {} func TestService_DeleteRunTag_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.DeleteRunTagRequest @@ -795,8 +799,9 @@ func TestService_GetRun_Ok(t *testing.T) { }, }, run.Metrics) } + func TestService_GetRun_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.GetRunRequest @@ -936,8 +941,9 @@ func TestService_LogBatch_Ok(t *testing.T) { // compare results. assert.Nil(t, err) } + func TestService_LogBatch_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.LogBatchRequest @@ -1324,8 +1330,9 @@ func TestService_LogMetric_Ok(t *testing.T) { // compare results. assert.Nil(t, err) } + func TestService_LogMetric_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.LogMetricRequest @@ -1530,8 +1537,9 @@ func TestService_LogParam_Ok(t *testing.T) { // compare results. assert.Nil(t, err) } + func TestService_LogParam_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.LogParamRequest diff --git a/pkg/api/mlflow/service/run/validators_test.go b/pkg/api/mlflow/service/run/validators_test.go index b9551a741..d5d545112 100644 --- a/pkg/api/mlflow/service/run/validators_test.go +++ b/pkg/api/mlflow/service/run/validators_test.go @@ -16,8 +16,9 @@ func TestValidateUpdateRunRequest_Ok(t *testing.T) { }) assert.Nil(t, err) } + func TestValidateUpdateRunRequest_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.UpdateRunRequest @@ -43,8 +44,9 @@ func TestValidateGetRunRequest_Ok(t *testing.T) { }) assert.Nil(t, err) } + func TestValidateGetRunRequest_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.GetRunRequest @@ -70,8 +72,9 @@ func TestValidateDeleteRunRequest_Ok(t *testing.T) { }) assert.Nil(t, err) } + func TestValidateDeleteRunRequest_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.DeleteRunRequest @@ -97,8 +100,9 @@ func TestValidateRestoreRunRequest_Ok(t *testing.T) { }) assert.Nil(t, err) } + func TestValidateRestoreRunRequest_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.RestoreRunRequest @@ -127,8 +131,9 @@ func TestValidateLogMetricRequest_Ok(t *testing.T) { }) assert.Nil(t, err) } + func TestValidateLogMetricRequest_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.LogMetricRequest @@ -171,8 +176,9 @@ func TestValidateLogParamRequest_Ok(t *testing.T) { }) assert.Nil(t, err) } + func TestValidateLogParamRequest_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.LogParamRequest @@ -207,8 +213,9 @@ func TestValidateSetRunTagRequest_Ok(t *testing.T) { }) assert.Nil(t, err) } + func TestValidateSetRunTagRequest_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.SetRunTagRequest @@ -241,8 +248,9 @@ func TestValidateDeleteRunTagRequest_Ok(t *testing.T) { }) assert.Nil(t, err) } + func TestValidateDeleteRunTagRequest_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.DeleteRunTagRequest @@ -268,8 +276,9 @@ func TestValidateLogBatchRequest_Ok(t *testing.T) { }) assert.Nil(t, err) } + func TestValidateLogBatchRequest_Error(t *testing.T) { - var testData = []struct { + testData := []struct { name string error *api.ErrorResponse request *request.LogBatchRequest @@ -296,9 +305,9 @@ func TestValidateSearchRunsRequest_Ok(t *testing.T) { }) assert.Nil(t, err) } -func TestValidateSearchRunsRequest_Error(t *testing.T) { - var testData = []struct { +func TestValidateSearchRunsRequest_Error(t *testing.T) { + testData := []struct { name string error *api.ErrorResponse request *request.SearchRunsRequest diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index a91c98559..5d39a4626 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -4,11 +4,11 @@ import ( "fmt" "strings" - "github.com/G-Research/fasttrackml/pkg/version" - log "github.com/sirupsen/logrus" "github.com/spf13/cobra" "github.com/spf13/viper" + + "github.com/G-Research/fasttrackml/pkg/version" ) const ( diff --git a/tests/integration/golang/fixtures/run.go b/tests/integration/golang/fixtures/run.go index c0b8892bf..26b83e92c 100644 --- a/tests/integration/golang/fixtures/run.go +++ b/tests/integration/golang/fixtures/run.go @@ -75,7 +75,8 @@ func (f RunFixtures) CreateRuns( // GetTestRuns fetches all runs for an experiment func (f RunFixtures) GetTestRuns( - ctx context.Context, experimentID int32) ([]models.Run, error) { + ctx context.Context, experimentID int32, +) ([]models.Run, error) { var runs []models.Run if err := f.db.WithContext(ctx).Where( "experiment_id = ?", experimentID, @@ -91,7 +92,8 @@ func (f RunFixtures) GetTestRuns( // FindMinMaxRowNums finds min and max rownum for an experiment's runs func (f RunFixtures) FindMinMaxRowNums( - ctx context.Context, experimentID int32) (int64, int64, error) { + ctx context.Context, experimentID int32, +) (int64, int64, error) { runs, err := f.GetTestRuns(ctx, experimentID) if err != nil { return 0, 0, eris.Wrap(err, "error fetching test runs")