Migrated `databricks_cluster` resource to Go SDK #3376

tanmay-db · 2024-03-18T03:23:16Z

Changes

Migrated cluster resource to Go SDK along with migrating major references of libraries api and method across clusters, jobs, exporter etc.

Note: Many resources use old clusters API, methods -- migration for these would be part of when we will be migrating those resources to Go SDK to account for resource specific OpenAPI changes, extra methods at that time like we did with the libraries resource.

Tests

Unit tests, Nightly tests are running ...

make test run locally
relevant change in docs/ folder
covered with integration tests in internal/acceptance
relevant acceptance tests are passing
using Go SDK

…ricks into migrate-cluster

codecov-commenter · 2024-03-18T03:26:47Z

Codecov Report

Attention: Patch coverage is 75.42857% with 43 lines in your changes are missing coverage. Please review.

Project coverage is 82.95%. Comparing base (3d30621) to head (cbcfc68).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3376      +/-   ##
==========================================
- Coverage   83.51%   82.95%   -0.56%     
==========================================
  Files         178      178              
  Lines       16595    16575      -20     
==========================================
- Hits        13859    13750     -109     
- Misses       1891     1967      +76     
- Partials      845      858      +13

Files	Coverage Δ
jobs/resource_job.go	`89.45% <ø> (ø)`
pipelines/resource_pipeline.go	`92.85% <ø> (ø)`
exporter/importables.go	`79.84% <80.00%> (ø)`
libraries/libraries_api.go	`56.09% <42.85%> (-38.81%)`	⬇️
clusters/resource_cluster.go	`85.26% <84.80%> (-2.00%)`	⬇️
exporter/util.go	`74.55% <50.00%> (-2.24%)`	⬇️

... and 1 file with indirect coverage changes

…ricks into migrate-cluster

libraries/libraries_api.go

clusters/resource_cluster.go

mgyucht

Some comments, but otherwise looking good!

clusters/resource_cluster.go

clusters/resource_cluster_test.go

clusters/resource_cluster.go

libraries/libraries_api.go

clusters/resource_cluster.go

…ricks into migrate-cluster

tanmay-db · 2024-04-11T13:30:40Z

clusters/resource_cluster.go

+	}
+}
+
+type LibraryList struct {


We need this type (with struct annotations) below, so we have to explicitly declare it.

var libraryList LibraryList common.DataToStructPointer(d, clusterSchema, &libraryList)

Given it's only used in clusters and to not encourage wide use, I think we should keep it here as compared to libraries_api.

cc: @miles, I am not sure if there is a better / easier way to do this, we could probably get the type from ClusterSpec.Libraries using reflection but I think having a 2 line struct would be simpler.

Updated to use ClusterSpec

mgyucht

Two very small nits, but otherwise this LGTM!

mgyucht · 2024-04-12T07:44:54Z

clusters/resource_cluster.go

+	var libraryList compute.InstallLibraries
 	common.DataToStructPointer(d, clusterSchema, &libraryList)


The point of adding libraries to the schema was that we don't need to call DataToStructPointer multiple times, right?

Suggested change

var libraryList compute.InstallLibraries

common.DataToStructPointer(d, clusterSchema, &libraryList)

libraryList := compute.InstallLibraries{

ClusterId: clusterInfo.ClusterId,

Libraries: cluster.Libraries,

}

mgyucht · 2024-04-12T07:47:37Z

clusters/resource_cluster.go

+	var libraryList ClusterSpec
+	common.DataToStructPointer(d, clusterSchema, &libraryList)
+	libsToInstall, libsToUninstall := libraries.GetLibrariesToInstallAndUninstall(libraryList.Libraries, libsClusterStatus)


This as well?

Suggested change

var libraryList ClusterSpec

common.DataToStructPointer(d, clusterSchema, &libraryList)

libsToInstall, libsToUninstall := libraries.GetLibrariesToInstallAndUninstall(libraryList.Libraries, libsClusterStatus)

libsToInstall, libsToUninstall := libraries.GetLibrariesToInstallAndUninstall(cluster.Libraries, libsClusterStatus)

tanmay-db · 2024-04-12T09:09:21Z

Going ahead with merge for now, need to check issues arising due to differences between ClusterSpec and other Cluster structs.

tanmay-db added 10 commits March 15, 2024 15:44

-

ef90181

-

ae09a53

-

dc3d19e

-

3d92135

-

85e0d3e

-

e693918

-

b12e8e8

passing clusters unit test

97d4de8

passing clusters integration test

41ddd03

Remove and fix libraries in jobs, pipelines, exporter

1dc63ea

tanmay-db requested review from a team as code owners March 18, 2024 03:23

tanmay-db requested review from mgyucht and removed request for a team and mgyucht March 18, 2024 03:23

Merge branch 'main' of github.com:databricks/terraform-provider-datab…

d66d953

…ricks into migrate-cluster

tanmay-db changed the title ~~Migrated databricks_cluster resource to Go SDK~~ [WIP] Migrated databricks_cluster resource to Go SDK Mar 18, 2024

alexott requested review from a team, mgyucht and edwardfeng-db March 18, 2024 08:47

tanmay-db added 8 commits March 18, 2024 19:19

cleanup

634fbc1

All unit tests passing

9e31da7

All unit tests passing

279f60e

Merge branch 'main' of github.com:databricks/terraform-provider-datab…

16062ce

…ricks into migrate-cluster

-

4e8afc0

cleanup

ebe5124

Merge branch 'main' of github.com:databricks/terraform-provider-datab…

ae843d7

…ricks into migrate-cluster

use wrapper in meantime go sdk change is merged

17956cd

Merge branch 'main' of github.com:databricks/terraform-provider-datab…

cbcfc68

…ricks into migrate-cluster

tanmay-db commented Mar 28, 2024

View reviewed changes

libraries/libraries_api.go Outdated Show resolved Hide resolved

tanmay-db commented Mar 28, 2024

View reviewed changes

clusters/resource_cluster.go Outdated Show resolved Hide resolved

tanmay-db changed the title ~~[WIP] Migrated databricks_cluster resource to Go SDK~~ Migrated databricks_cluster resource to Go SDK Mar 28, 2024

tanmay-db requested review from alexott and nkvuong March 28, 2024 12:26

mgyucht reviewed Mar 28, 2024

View reviewed changes

tanmay-db added 6 commits April 2, 2024 20:06

Merge branch 'main' of github.com:databricks/terraform-provider-datab…

b83b2eb

…ricks into migrate-cluster

upd to main

78634d8

comments

1659589

Merge branch 'main' of github.com:databricks/terraform-provider-datab…

ef902ff

…ricks into migrate-cluster

comments

652ee69

-

c1e8a3d

tanmay-db requested a review from mgyucht April 11, 2024 11:26

tanmay-db added 2 commits April 11, 2024 18:20

-

b670d4c

-

6278c65

tanmay-db commented Apr 11, 2024

View reviewed changes

-

4d88b38

mgyucht approved these changes Apr 12, 2024

View reviewed changes

tanmay-db added this pull request to the merge queue Apr 12, 2024

Merged via the queue into main with commit f2e395b Apr 12, 2024
5 checks passed

tanmay-db deleted the migrate-cluster branch April 12, 2024 09:13

tanmay-db mentioned this pull request Apr 16, 2024

Release v1.40.0 #3473

Merged

alexott mentioned this pull request May 8, 2024

[ISSUE] Issue with databricks_cluster resource using exporter does not include cluster libraries #3558

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrated `databricks_cluster` resource to Go SDK #3376

Migrated `databricks_cluster` resource to Go SDK #3376

tanmay-db commented Mar 18, 2024 •

edited by alexott

Loading

codecov-commenter commented Mar 18, 2024 •

edited

Loading

mgyucht left a comment

tanmay-db Apr 11, 2024

tanmay-db Apr 11, 2024

tanmay-db Apr 11, 2024

mgyucht left a comment

mgyucht Apr 12, 2024

mgyucht Apr 12, 2024

tanmay-db commented Apr 12, 2024

		var libraryList compute.InstallLibraries
		common.DataToStructPointer(d, clusterSchema, &libraryList)

-	var libraryList compute.InstallLibraries
-	common.DataToStructPointer(d, clusterSchema, &libraryList)
+	libraryList := compute.InstallLibraries{
+	    ClusterId: clusterInfo.ClusterId,
+	    Libraries: cluster.Libraries,
+	}

Migrated databricks_cluster resource to Go SDK #3376

Migrated databricks_cluster resource to Go SDK #3376

Conversation

tanmay-db commented Mar 18, 2024 • edited by alexott Loading

Changes

Tests

codecov-commenter commented Mar 18, 2024 • edited Loading

Codecov Report

mgyucht left a comment

Choose a reason for hiding this comment

tanmay-db Apr 11, 2024

Choose a reason for hiding this comment

tanmay-db Apr 11, 2024

Choose a reason for hiding this comment

tanmay-db Apr 11, 2024

Choose a reason for hiding this comment

mgyucht left a comment

Choose a reason for hiding this comment

mgyucht Apr 12, 2024

Choose a reason for hiding this comment

mgyucht Apr 12, 2024

Choose a reason for hiding this comment

tanmay-db commented Apr 12, 2024

Migrated `databricks_cluster` resource to Go SDK #3376

Migrated `databricks_cluster` resource to Go SDK #3376

tanmay-db commented Mar 18, 2024 •

edited by alexott

Loading

codecov-commenter commented Mar 18, 2024 •

edited

Loading