[Feature] Refactor databricks_mlflow_experiment to Go SDK (#4606)

## Changes
- Clean up to use structs from go sdk
- Correct the attributes (`tags` is missing and `description` does not
exist)
- Suppress diff in `databricks_mlflow_experiment` name, close #4601

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->

- [x] `make test` run locally
- [x] covered with integration tests in `internal/acceptance`
- [x] using Go SDK

---------

Co-authored-by: Alex Ott <[email protected]>

Author: nkvuong

NEXT_CHANGELOG.md

@@ -6,6 +6,8 @@
 
 ### Bug Fixes
 
+ * Suppress diff in `databricks_mlflow_experiment` name ([#4606](https://github.com/databricks/terraform-provider-databricks/pull/4606))
+
 ### Documentation
 
  * Add import instructions for `databricks_share` and `databricks_recipient` ([#4608](https://github.com/databricks/terraform-provider-databricks/pull/4608))

docs/resources/mlflow_experiment.md

@@ -13,7 +13,15 @@ data "databricks_current_user" "me" {}
 resource "databricks_mlflow_experiment" "this" {
   name              = "${data.databricks_current_user.me.home}/Sample"
   artifact_location = "dbfs:/tmp/my-experiment"
-  description       = "My MLflow experiment description"
+
+  tags {
+    key   = "key1"
+    value = "value1"
+  }
+  tags {
+    key   = "key2"
+    value = "value2"
+  }  
 }
 ```
 
@@ -23,7 +31,7 @@ The following arguments are supported:
 
 * `name` - (Required) Name of MLflow experiment. It must be an absolute path within the Databricks workspace, e.g. `/Users/<some-username>/my-experiment`. For more information about changes to experiment naming conventions, see [mlflow docs](https://docs.databricks.com/applications/mlflow/experiments.html#experiment-migration).
 * `artifact_location` - Path to dbfs:/ or s3:// artifact location of the MLflow experiment.
-* `description` - The description of the MLflow experiment.
+* `tags` - Tags for the MLflow experiment.
 
 ## Attribute Reference
 

mlflow/mlflow_experiment_test.go

@@ -12,7 +12,17 @@ func TestAccMLflowExperiment(t *testing.T) {
 		resource "databricks_mlflow_experiment" "e1" {
 			name = "/Shared/tf-{var.RANDOM}"
 			artifact_location = "dbfs:/tmp/tf-{var.RANDOM}"
-			description = "tf-{var.RANDOM} description"
+		}
+		`,
+	})
+}
+
+func TestAccMLflowExperimentWithDiff(t *testing.T) {
+	acceptance.WorkspaceLevel(t, acceptance.Step{
+		Template: `
+		resource "databricks_mlflow_experiment" "e1" {
+			name = "/Workspace/Shared/tf-{var.RANDOM}/"
+			artifact_location = "dbfs:/tmp/tf-{var.RANDOM}"
 		}
 		`,
 	})

mlflow/resource_mlflow_experiment.go

@@ -2,101 +2,83 @@ package mlflow
 
 import (
 	"context"
+	"log"
+	"strings"
 
+	"github.com/databricks/databricks-sdk-go/service/ml"
 	"github.com/databricks/terraform-provider-databricks/common"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
 
-// Experiment defines the response object from the API
-type Experiment struct {
-	Name             string `json:"name"`
-	Description      string `json:"description,omitempty"`
-	ExperimentId     string `json:"experiment_id,omitempty" tf:"computed"`
-	ArtifactLocation string `json:"artifact_location,omitempty" tf:"force_new,suppress_diff"`
-	LifecycleStage   string `json:"lifecycle_stage,omitempty" tf:"computed"`
-	LastUpdateTime   int64  `json:"last_update_time,omitempty" tf:"computed"`
-	CreationTime     int64  `json:"creation_time,omitempty" tf:"computed"`
-}
-
-type experimentUpdate struct {
-	ExperimentId string `json:"experiment_id"`
-	NewName      string `json:"new_name"`
-}
-
-type experimentWrapper struct {
-	Experiment Experiment `json:"experiment"`
-}
-
-// ExperimentsAPI ...
-type ExperimentsAPI struct {
-	client  *common.DatabricksClient
-	context context.Context
-}
+func experimentNameSuppressDiff(k, old, new string, d *schema.ResourceData) bool {
 
-// NewExperimentsAPI ...
-func NewExperimentsAPI(ctx context.Context, m any) ExperimentsAPI {
-	return ExperimentsAPI{m.(*common.DatabricksClient), ctx}
-}
-
-// Create ...
-func (a ExperimentsAPI) Create(d *Experiment) error {
-	return a.client.Post(a.context, "/mlflow/experiments/create", d, &d)
-}
-
-// Read ...
-func (a ExperimentsAPI) Read(experimentId string) (*Experiment, error) {
-	var d experimentWrapper
-	err := a.client.Get(a.context, "/mlflow/experiments/get", map[string]string{
-		"experiment_id": experimentId,
-	}, &d)
-	if err != nil {
-		return nil, err
+	if strings.TrimSuffix(strings.TrimPrefix(new, "/Workspace"), "/") == strings.TrimSuffix(strings.TrimPrefix(old, "/Workspace"), "/") {
+		log.Printf("[DEBUG] Ignoring configuration drift from %s to %s", old, new)
+		return true
 	}
-	return &d.Experiment, nil
-}
-
-// Update ...
-func (a ExperimentsAPI) Update(e *experimentUpdate) error {
-	return a.client.Post(a.context, "/mlflow/experiments/update", e, &e)
-}
-
-// Delete ...
-func (a ExperimentsAPI) Delete(id string) error {
-	return a.client.Post(a.context, "/mlflow/experiments/delete", map[string]string{
-		"experiment_id": id,
-	}, nil)
+	return false
 }
 
 func ResourceMlflowExperiment() common.Resource {
 	s := common.StructToSchema(
-		Experiment{},
-		common.NoCustomize)
+		ml.Experiment{},
+		func(m map[string]*schema.Schema) map[string]*schema.Schema {
+			for _, p := range []string{"creation_time", "experiment_id", "last_update_time", "lifecycle_stage", "tags"} {
+				common.CustomizeSchemaPath(m, p).SetComputed()
+			}
+			common.CustomizeSchemaPath(m, "artifact_location").SetForceNew().SetSuppressDiff()
+			common.CustomizeSchemaPath(m, "name").SetRequired().SetCustomSuppressDiff(experimentNameSuppressDiff)
+			// the API never accepts description, but we need to keep this for backwards compatibility
+			m["description"] = &schema.Schema{
+				Optional:   true,
+				Type:       schema.TypeString,
+				Deprecated: "Remove the description attribute as it no longer is used and will be removed in a future version.",
+			}
+			return m
+		})
 
 	return common.Resource{
 		Create: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
-			var e Experiment
-			common.DataToStructPointer(d, s, &e)
-			if err := NewExperimentsAPI(ctx, c).Create(&e); err != nil {
+			w, err := c.WorkspaceClient()
+			if err != nil {
+				return err
+			}
+			var create ml.CreateExperiment
+			common.DataToStructPointer(d, s, &create)
+			response, err := w.Experiments.CreateExperiment(ctx, create)
+			if err != nil {
 				return err
 			}
-			d.SetId(e.ExperimentId)
+			d.SetId(response.ExperimentId)
 			return nil
 		},
 		Read: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
-			e, err := NewExperimentsAPI(ctx, c).Read(d.Id())
+			w, err := c.WorkspaceClient()
 			if err != nil {
 				return err
 			}
-			return common.StructToData(*e, s, d)
+			e, err := w.Experiments.GetExperiment(ctx, ml.GetExperimentRequest{ExperimentId: d.Id()})
+			if err != nil {
+				return err
+			}
+			return common.StructToData(e.Experiment, s, d)
 		},
 		Update: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
-			var e Experiment
-			common.DataToStructPointer(d, s, &e)
-			updateDoc := experimentUpdate{ExperimentId: d.Id(), NewName: e.Name}
-			return NewExperimentsAPI(ctx, c).Update(&updateDoc)
+			w, err := c.WorkspaceClient()
+			if err != nil {
+				return err
+			}
+			return w.Experiments.UpdateExperiment(ctx, ml.UpdateExperiment{
+				ExperimentId: d.Id(),
+				NewName:      d.Get("name").(string),
+			})
 		},
 		Delete: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
-			return NewExperimentsAPI(ctx, c).Delete(d.Id())
+			w, err := c.WorkspaceClient()
+			if err != nil {
+				return err
+			}
+			return w.Experiments.DeleteExperiment(ctx, ml.DeleteExperiment{ExperimentId: d.Id()})
 		},
 		StateUpgraders: []schema.StateUpgrader{},
 		Schema:         s,

mlflow/resource_mlflow_experiment_test.go

@@ -3,12 +3,13 @@ package mlflow
 import (
 	"testing"
 
+	"github.com/databricks/databricks-sdk-go/service/ml"
 	"github.com/databricks/terraform-provider-databricks/qa"
 	"github.com/stretchr/testify/assert"
 )
 
-func e() Experiment {
-	return Experiment{
+func e() ml.Experiment {
+	return ml.Experiment{
 		Name: "xyz",
 	}
 }
@@ -27,8 +28,8 @@ func TestExperimentCreate(t *testing.T) {
 			{
 				Method:   "GET",
 				Resource: "/api/2.0/mlflow/experiments/get?experiment_id=123456790123456",
-				Response: experimentWrapper{
-					Experiment: re,
+				Response: ml.GetExperimentResponse{
+					Experiment: &re,
 				},
 			},
 		},
@@ -81,8 +82,8 @@ func TestExperimentCreateGetError(t *testing.T) {
 			{
 				Method:   "GET",
 				Resource: "/api/2.0/mlflow/experiments/get?experiment_id=123456790123456",
-				Response: experimentWrapper{
-					Experiment: re,
+				Response: ml.GetExperimentResponse{
+					Experiment: &re,
 				},
 				Status: 400,
 			},
@@ -105,8 +106,8 @@ func TestExperimentRead(t *testing.T) {
 			{
 				Method:   "GET",
 				Resource: "/api/2.0/mlflow/experiments/get?experiment_id=123456790123456",
-				Response: experimentWrapper{
-					Experiment: re,
+				Response: ml.GetExperimentResponse{
+					Experiment: &re,
 				},
 			},
 		},
@@ -127,8 +128,8 @@ func TestExperimentReadGetError(t *testing.T) {
 			{
 				Method:   "GET",
 				Resource: "/api/2.0/mlflow/experiments/get?experiment_id=123456790123456",
-				Response: experimentWrapper{
-					Experiment: re,
+				Response: ml.GetExperimentResponse{
+					Experiment: &re,
 				},
 				Status: 400,
 			},
@@ -142,17 +143,17 @@ func TestExperimentReadGetError(t *testing.T) {
 }
 
 func TestExperimentUpdate(t *testing.T) {
-	resPost := Experiment{
+	resPost := ml.Experiment{
 		ExperimentId: "123456790123456",
 		Name:         "123",
 	}
-	d, err := qa.ResourceFixture{
+	qa.ResourceFixture{
 		Fixtures: []qa.HTTPFixture{
 			{
 				Method:   "GET",
 				Resource: "/api/2.0/mlflow/experiments/get?experiment_id=123456790123456",
-				Response: experimentWrapper{
-					Experiment: resPost,
+				Response: ml.GetExperimentResponse{
+					Experiment: &resPost,
 				},
 			},
 			{
@@ -167,15 +168,14 @@ func TestExperimentUpdate(t *testing.T) {
 		HCL: `
 		name = "123"
 		`,
-	}.Apply(t)
-
-	assert.NoError(t, err)
-	assert.Equal(t, resPost.ExperimentId, d.Id(), "Resource ID should not be empty")
-	assert.Equal(t, resPost.Name, d.Get("name"), "Name should be updated")
+	}.ApplyAndExpectData(t, map[string]any{
+		"name": resPost.Name,
+		"id":   resPost.ExperimentId,
+	})
 }
 
 func TestExperimentUpdatePostError(t *testing.T) {
-	resPost := Experiment{
+	resPost := ml.Experiment{
 		ExperimentId: "123456790123456",
 		Name:         "123",
 	}
@@ -203,7 +203,7 @@ func TestExperimentDelete(t *testing.T) {
 	r := map[string]string{
 		"experiment_id": "123456790123456",
 	}
-	d, err := qa.ResourceFixture{
+	qa.ResourceFixture{
 		Fixtures: []qa.HTTPFixture{
 			{
 				Method:          "POST",
@@ -217,10 +217,7 @@ func TestExperimentDelete(t *testing.T) {
 		HCL: `
 		name = "xyz"
 		`,
-	}.Apply(t)
-
-	assert.NoError(t, err)
-	assert.Equal(t, r["experiment_id"], d.Id(), "Resource ID should not be empty")
+	}.ApplyNoError(t)
 }
 
 func TestExperimentDeleteError(t *testing.T) {
@@ -246,3 +243,10 @@ func TestExperimentDeleteError(t *testing.T) {
 
 	assert.Error(t, err)
 }
+
+func TestExperimentNameSuppressDiff(t *testing.T) {
+	assert.True(t, experimentNameSuppressDiff("", "/Workspace/EXPERIMENT/", "/EXPERIMENT", nil))
+	assert.True(t, experimentNameSuppressDiff("", "/Workspace/EXPERIMENT", "/EXPERIMENT", nil))
+	assert.True(t, experimentNameSuppressDiff("", "/EXPERIMENT/", "/EXPERIMENT", nil))
+	assert.False(t, experimentNameSuppressDiff("", "/new_name", "/EXPERIMENT/", nil))
+}