-
Notifications
You must be signed in to change notification settings - Fork 488
Add support for default_workspace_id #5284
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add support for default_workspace_id #5284
Conversation
f30f724 to
d486bb5
Compare
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
tanmay-db
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments. Could you confirm if this works E2E manually on jobs resource. Also we would need to update the provider documentation. See: https://registry.terraform.io/providers/databricks/databricks/latest/docs#argument-reference
| // defaultWorkspaceID is the default workspace ID to use when workspace_id is not | ||
| // specified in provider_config at the resource level. This is set from the provider | ||
| // configuration and serves as a fallback for unified provider resources. | ||
| defaultWorkspaceID string | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: These changes would need to be done upstream as this file is generated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will add it to the generated file template
| // Add default_workspace_id for unified provider support | ||
| ps["default_workspace_id"] = &schema.Schema{ | ||
| Type: schema.TypeString, | ||
| Optional: true, | ||
| ValidateFunc: func(val interface{}, key string) (warns []string, errs []error) { | ||
| v := val.(string) | ||
| if v == "" { | ||
| return | ||
| } | ||
| if !regexp.MustCompile(`^[1-9]\d*$`).MatchString(v) { | ||
| errs = append(errs, fmt.Errorf("default_workspace_id must be a positive integer without leading zeros")) | ||
| } | ||
| return | ||
| }, | ||
| Description: "Default workspace ID to use when workspace_id is not specified in provider_config at the resource level", | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to add this separately or can this be part of ConfigAttributes like it's done for all the other attributes. See line above 316
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ConfigAttributes come from config in Go SDK. default_workspace_id only makes sense in the context of terraform provider so in my opinion, it should not be the part of Go SDK config.
Changes
Added support for default workspace ID in plugin framework and sdkv2 configure methods. A new defaultWorkspaceID field is a part of DatabricksClient.
Tests
Unit test.
make testrun locallydocs/folderinternal/acceptanceNEXT_CHANGELOG.mdfile