Skip to content

[Fix] Update databricks_mlflow_experiment resource to include tags#4569

Closed
tanmay-db wants to merge 4 commits intomainfrom
mlflow-schema
Closed

[Fix] Update databricks_mlflow_experiment resource to include tags#4569
tanmay-db wants to merge 4 commits intomainfrom
mlflow-schema

Conversation

@tanmay-db
Copy link
Copy Markdown
Contributor

@tanmay-db tanmay-db commented Mar 11, 2025

Changes

This resource is outdated as it is maintained manually. PR adds the tags field which is present in OpenAPI spec and removes the description field which isn't present.

The right way to solve this is to migrate this to Go SDK however since the bandwidth is low for this quarter to include that work and this is required in CLI, adding a quick fix until this is migrated to autogeneration framework.

Tests

Added unit test

  • make test run locally
  • relevant change in docs/ folder
  • covered with integration tests in internal/acceptance
  • using Go SDK
  • using TF Plugin Framework

@tanmay-db tanmay-db requested review from a team as code owners March 11, 2025 17:31
@tanmay-db tanmay-db requested review from mgyucht and removed request for a team March 11, 2025 17:31
@tanmay-db tanmay-db temporarily deployed to test-trigger-is March 11, 2025 17:31 — with GitHub Actions Inactive
@tanmay-db tanmay-db changed the title [Fix] Update resource to include tags [Fix] Update databricks_mlflow_experiment resource to include tags Mar 11, 2025
@tanmay-db tanmay-db removed request for a team and mgyucht March 11, 2025 17:32
@tanmay-db tanmay-db temporarily deployed to test-trigger-is March 11, 2025 17:32 — with GitHub Actions Inactive
@tanmay-db tanmay-db temporarily deployed to test-trigger-is March 11, 2025 17:33 — with GitHub Actions Inactive
Copy link
Copy Markdown
Contributor

@alexott alexott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in general good, just not sure that this is a fix

### New Features and Improvements

### Bug Fixes
* [Fix] Update databricks_mlflow_experiment resource to include tags ([#4569](https://github.com/databricks/terraform-provider-databricks/pull/4569))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say that this "new feature" not a fix

@alexott alexott requested a review from Copilot March 11, 2025 18:27
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR updates the databricks_mlflow_experiment resource so that it includes a tags field per the OpenAPI spec while removing the outdated description field.

  • Added support for tags in the Experiment resource and corresponding tests
  • Removed the description field from the resource schema
  • Updated documentation and changelog to reflect the new resource structure

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
mlflow/resource_mlflow_experiment_test.go Added a new test to verify experiment creation with tags
NEXT_CHANGELOG.md Updated changelog with a fix entry for the tags implementation
docs/resources/mlflow_experiment.md Updated resource documentation to replace description with tags
mlflow/resource_mlflow_experiment.go Modified the Experiment struct: removed description and added tags
Comments suppressed due to low confidence (1)

mlflow/resource_mlflow_experiment_test.go:95

  • Consider adding an assertion to verify that the 'tags' field in the resource state is correctly populated.
assert.Equal(t, re.Name, d.Get("name"), "Experiment name should be set")

@github-actions
Copy link
Copy Markdown

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/terraform

Inputs:

  • PR number: 4569
  • Commit SHA: c6317e95a725f975eddfd914f788508647577921

Checks will be approved automatically on success.

@nkvuong
Copy link
Copy Markdown
Contributor

nkvuong commented Apr 2, 2025

Superseded by #4606

@nkvuong nkvuong closed this Apr 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants