Ignore test_log_batch_null_metrics (#48)

* Ignore test_log_batch_null_metrics

Signed-off-by: nojaf <florian.verdonck@outlook.com>

* Ignore test_sqlalchemy_store_behaves_as_expected_with_inmemory_sqlite_db

Signed-off-by: nojaf <florian.verdonck@outlook.com>

* Update readme

Signed-off-by: nojaf <florian.verdonck@outlook.com>

* Address linting issue

Signed-off-by: nojaf <florian.verdonck@outlook.com>

---------

Signed-off-by: nojaf <florian.verdonck@outlook.com>

README.md

@@ -278,11 +278,9 @@ Sometimes `golangci-lint` can complain about unrelated files, run `golangci-lint
 The following Python tests are currently failing:
 
 ```
-======================================================================================== short test summary info ========================================================================================
-FAILED .mlflow.repo/tests/store/tracking/test_sqlalchemy_store.py::test_log_batch_null_metrics - TypeError: must be real number, not NoneType
+===================================================================================================================== short test summary info ======================================================================================================================
 FAILED .mlflow.repo/tests/store/tracking/test_sqlalchemy_store.py::test_log_inputs_with_large_inputs_limit_check - AssertionError: assert {'digest': 'd...ema': '', ...} == {'digest': 'd...a': None, ...}
-FAILED .mlflow.repo/tests/store/tracking/test_sqlalchemy_store.py::test_sqlalchemy_store_behaves_as_expected_with_inmemory_sqlite_db - mlflow.exceptions.MlflowException: failed to create experiment
-=========================================================== 3 failed, 356 passed, 9 skipped, 128 deselected, 10 warnings in 429.04s (0:07:09) ===========================================================
+======================================================================================== 1 failed, 358 passed, 9 skipped, 128 deselected, 10 warnings in 227.64s (0:03:47) =========================================================================================
 ```
 
 ## Debug Failing Tests

conftest.py

@@ -39,6 +39,19 @@ def pytest_configure(config):
             "tests.store.tracking.test_sqlalchemy_store.test_log_batch_params_max_length_value",
             "tests/override_test_sqlalchemy_store.py",
         ),
+        # This tests calls the store using invalid metric entity that cannot be converted
+        # to its proto counterpart.
+        # Example: entities.Metric("invalid_metric", None, (int(time.time() * 1000)), 0).to_proto()
+        (
+            "tests.store.tracking.test_sqlalchemy_store.test_log_batch_null_metrics",
+            "tests/override_test_sqlalchemy_store.py",
+        ),
+        # We do not support applying the SQL schema to sqlite like Python does.
+        # So we do not support sqlite:////:memory: database.
+        (
+            "tests.store.tracking.test_sqlalchemy_store.test_sqlalchemy_store_behaves_as_expected_with_inmemory_sqlite_db",
+            "tests/override_test_sqlalchemy_store.py",
+        ),
     ):
         func_name = func_to_patch.rsplit(".", 1)[1]
         new_func_file = (

pkg/sql/sql.go

@@ -2,6 +2,7 @@ package sql
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/url"
 	"strings"
@@ -15,33 +16,61 @@ import (
 	"github.com/mlflow/mlflow-go/pkg/utils"
 )
 
-func NewDatabase(ctx context.Context, storeURL string) (*gorm.DB, error) {
-	logger := utils.GetLoggerFromContext(ctx)
-
-	uri, err := url.Parse(storeURL)
-	if err != nil {
-		return nil, fmt.Errorf("failed to parse store URL %q: %w", storeURL, err)
-	}
-
-	var dialector gorm.Dialector
+var errSqliteMemory = errors.New("go implementation does not support :memory: for sqlite")
 
+//nolint:ireturn
+func getDialector(uri *url.URL) (gorm.Dialector, error) {
 	uri.Scheme, _, _ = strings.Cut(uri.Scheme, "+")
 
 	switch uri.Scheme {
 	case "mssql":
 		uri.Scheme = "sqlserver"
-		dialector = sqlserver.Open(uri.String())
+
+		return sqlserver.Open(uri.String()), nil
 	case "mysql":
-		dialector = mysql.Open(fmt.Sprintf("%s@tcp(%s)%s?%s", uri.User, uri.Host, uri.Path, uri.RawQuery))
+		return mysql.Open(fmt.Sprintf("%s@tcp(%s)%s?%s", uri.User, uri.Host, uri.Path, uri.RawQuery)), nil
 	case "postgres", "postgresql":
-		dialector = postgres.Open(uri.String())
+		return postgres.Open(uri.String()), nil
 	case "sqlite":
 		uri.Scheme = ""
 		uri.Path = uri.Path[1:]
-		dialector = sqlite.Open(uri.String())
+
+		if uri.Path == ":memory:" {
+			return nil, errSqliteMemory
+		}
+
+		return sqlite.Open(uri.String()), nil
 	default:
 		return nil, fmt.Errorf("unsupported store URL scheme %q", uri.Scheme) //nolint:err113
 	}
+}
+
+func initSqlite(database *gorm.DB) error {
+	database.Exec("PRAGMA case_sensitive_like = true;")
+
+	sqlDB, err := database.DB()
+	if err != nil {
+		return fmt.Errorf("failed to get database instance: %w", err)
+	}
+	// set SetMaxOpenConns to be 1 only in case of SQLite to avoid `database is locked`
+	// in case of parallel calls to some endpoints that use `transactions`.
+	sqlDB.SetMaxOpenConns(1)
+
+	return nil
+}
+
+func NewDatabase(ctx context.Context, storeURL string) (*gorm.DB, error) {
+	logger := utils.GetLoggerFromContext(ctx)
+
+	uri, err := url.Parse(storeURL)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse store URL %q: %w", storeURL, err)
+	}
+
+	dialector, err := getDialector(uri)
+	if err != nil {
+		return nil, err
+	}
 
 	database, err := gorm.Open(dialector, &gorm.Config{
 		TranslateError: true,
@@ -52,15 +81,9 @@ func NewDatabase(ctx context.Context, storeURL string) (*gorm.DB, error) {
 	}
 
 	if dialector.Name() == "sqlite" {
-		database.Exec("PRAGMA case_sensitive_like = true;")
-
-		sqlDB, err := database.DB()
-		if err != nil {
-			return nil, fmt.Errorf("failed to get database instance: %w", err)
+		if err := initSqlite(database); err != nil {
+			return nil, err
 		}
-		// set SetMaxOpenConns to be 1 only in case of SQLite to avoid `database is locked`
-		// in case of parallel calls to some endpoints that use `transactions`.
-		sqlDB.SetMaxOpenConns(1)
 	}
 
 	return database, nil

tests/override_test_sqlalchemy_store.py

@@ -7,3 +7,11 @@ def test_log_batch_internal_error(store: SqlAlchemyStore):
 
 def test_log_batch_params_max_length_value(store: SqlAlchemyStore, monkeypatch):
     ()
+
+
+def test_log_batch_null_metrics(store: SqlAlchemyStore):
+    ()
+
+
+def test_sqlalchemy_store_behaves_as_expected_with_inmemory_sqlite_db(monkeypatch):
+    ()