Jobs Methods GoSDK Migration #3577

edwardfeng-db · 2024-05-14T16:05:37Z

Changes

Migrated Read, Update, Create methods to the go-sdk migration for the jobs resource
- Updated cluster related methods to accept compute.ClusterSpec as a valid input in resource_cluster.go
- Created jobs_api_go_sdk.go to replicate go-sdk versions of necessary methods that were part of JobsApi
- Updated jobs manually written schema to be consistent with the go-sdk schema, mostly taking out some omitempty
- Updated the Read, Update, Create methods to have two branches, if it's api2.0, make it run the exact same code as before, otherwise, use the go-sdk to perform the same actions
- Updated unit tests to include some missing fields that are newly marked as required, such as task_key
- Updated doc about schema changes
- Removed autotermination_minutes from the cluster spec used by jobs because it is confirmed to be never useful for jobs

Tests

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

codecov-commenter · 2024-05-16T11:11:43Z

Codecov Report

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

Project coverage is 81.60%. Comparing base (2717d46) to head (3bda6d3).
Report is 5 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3577      +/-   ##
==========================================
- Coverage   82.32%   81.60%   -0.73%     
==========================================
  Files         190      192       +2     
  Lines       19324    19550     +226     
==========================================
+ Hits        15909    15954      +45     
- Misses       2483     2645     +162     
- Partials      932      951      +19

Files	Coverage Δ
common/reflect_resource.go	`80.56% <100.00%> (ø)`
jobs/resource_job.go	`83.08% <76.05%> (-8.48%)`	⬇️
clusters/resource_cluster.go	`71.17% <27.27%> (-5.64%)`	⬇️
jobs/jobs_api_go_sdk.go	`38.55% <38.55%> (ø)`

... and 4 files with indirect coverage changes

jobs/jobs_api_go_sdk.go

edwardfeng-db · 2024-05-22T16:25:12Z

Verified that nightly integration tests passed for this PR

clusters/resource_cluster.go

docs/resources/job.md

jobs/jobs_api_go_sdk.go

tanmay-db · 2024-05-23T12:04:58Z

jobs/jobs_api_go_sdk.go

+}
+
+// Removing unnecesary fields out of ClusterSpec's ForceSendFields because the current terraform plugin sdk
+// has a limitation that it will always set unspecified values to the zero value.


Some context on how the plugin framework could help in case of null vs zero:

SDKv2 also does not expose Terraform's null or unknown value concepts unambiguously, typically returning both null and unknown values as the same zero-value for the underlying Go type.

The framework type system fully exposes null, unknown, and known value states. You can reliably query each value with the IsNull() or IsUnknown() methods.

https://developer.hashicorp.com/terraform/plugin/framework/migrating/benefits

jobs/jobs_api_go_sdk.go

tanmay-db

Had some comments, LGTM

jobs/resource_job.go

jobs/jobs_api_go_sdk.go

edwardfeng-db · 2024-05-23T22:58:40Z

clusters/resource_cluster.go

+		Type:     schema.TypeInt,
+		Optional: true,
+		Default:  60,
+	})


Only adding it as a resource specific field for cluster because job resource never needs it

jobs/resource_job.go

clusters/resource_cluster.go

docs/resources/job.md

jobs/resource_job_test.go

nkvuong

have left a few questions regarding to changes in our unit tests, and also potential breaking changes for git provider, and some other optional -> required fields

edwardfeng-db · 2024-05-24T11:35:04Z

common/reflect_resource.go

@@ -617,7 +617,7 @@ func iterFields(rv reflect.Value, path []string, s map[string]*schema.Schema, al
 			return fmt.Errorf("inconsistency: %s has omitempty, but is not optional", fieldName)
 		}
 		defaultEmpty := reflect.ValueOf(fieldSchema.Default).Kind() == reflect.Invalid
-		if fieldSchema.Optional && defaultEmpty && !omitEmpty {
+		if !isGoSDK && fieldSchema.Optional && defaultEmpty && !omitEmpty {


Adding additional check because we are customizing git_provider to be optional while it doesn't have omitEmpty on the go-sdk struct.

jobs/resource_job_test.go

nkvuong

looks good to me, thanks for this

### New Features and Improvements * Docs: remove usage of deprecated `azurerm` options from PL guides ([#3606](#3606)). * Jobs Methods GoSDK Migration ([#3577](#3577)). * Do not suppress diff if it is explicitly changed to zero ([#3611](#3611)). * update markdown ([#3621](#3621)). * Fix resource_cluster bug with ebs volume fields ([#3613](#3613)). ### Documentation Changes ### Exporter ### Internal Changes

* Release v1.46.0 ### New Features and Improvements * Docs: remove usage of deprecated `azurerm` options from PL guides ([#3606](#3606)). * Jobs Methods GoSDK Migration ([#3577](#3577)). * Do not suppress diff if it is explicitly changed to zero ([#3611](#3611)). * update markdown ([#3621](#3621)). * Fix resource_cluster bug with ebs volume fields ([#3613](#3613)). ### Documentation Changes ### Exporter ### Internal Changes * - * -

edwardfeng-db force-pushed the edwardfeng-db/jobs-methods-migration-new branch 2 times, most recently from e0ac053 to 228d7ff Compare May 16, 2024 11:08

edwardfeng-db force-pushed the edwardfeng-db/jobs-methods-migration-new branch from 16dd0f0 to c1cb3af Compare May 21, 2024 14:39

edwardfeng-db requested review from hectorcast-db, alexott, nkvuong and tanmay-db May 21, 2024 15:39

edwardfeng-db marked this pull request as ready for review May 21, 2024 15:45

edwardfeng-db requested review from a team as code owners May 21, 2024 15:45

edwardfeng-db commented May 21, 2024

View reviewed changes

jobs/jobs_api_go_sdk.go Show resolved Hide resolved

edwardfeng-db force-pushed the edwardfeng-db/jobs-methods-migration-new branch from 4f71e36 to ef97830 Compare May 22, 2024 12:40

tanmay-db reviewed May 23, 2024

View reviewed changes

tanmay-db approved these changes May 23, 2024

View reviewed changes

jobs/resource_job.go Outdated Show resolved Hide resolved

jobs/jobs_api_go_sdk.go Outdated Show resolved Hide resolved

jobs/jobs_api_go_sdk.go Outdated Show resolved Hide resolved

edwardfeng-db commented May 23, 2024

View reviewed changes