Skip to content
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

Added databricks_sql_warehouses, databricks_sql_warehouse, and databricks_cluster data resources #1460

Merged
merged 7 commits into from
Jul 20, 2022

Conversation

nkvuong
Copy link
Contributor

@nkvuong nkvuong commented Jul 14, 2022

Add 3 new data sources: databricks_sql_warehouses (list all warehouses in a workspace, with a name filter), databricks_sql_warehouse and databricks_cluster (list attributes based on single id)

Close #1270, close #1456

@nkvuong nkvuong requested a review from nfx July 14, 2022 16:37
@alexott
Copy link
Contributor

alexott commented Jul 14, 2022

For cluster and warehouse it would be nice to search by name

DefaultTags map[string]string `json:"default_tags,omitempty" tf:"computed"`
ClusterLogStatus *LogSyncStatus `json:"cluster_log_status,omitempty" tf:"computed"`
TerminationReason *TerminationReason `json:"termination_reason,omitempty" tf:"computed"`
DataSecurityMode string `json:"data_security_mode,omitempty" tf:"computed"`
Copy link
Contributor

Choose a reason for hiding this comment

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

create new type for cluster data resource and probably just expose cluster info as a field. this is the least intrusive change.

)

func DataSourceCluster() *schema.Resource {

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: remove blank line

for_each = data.databricks_sql.warehouses.ids
id = each.value
}

Copy link
Contributor

Choose a reason for hiding this comment

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

run terrafmt on that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is formatted (I think)

return err
}
endpointsAPI := NewSQLEndpointsAPI(ctx, c)
data.DataSourceID, err = endpointsAPI.ResolveDataSourceID(data.ID)
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need to resolve a data resource if we have DataSourceID already? ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DataSourceID comes from a different API

func DataSourceWarehouses() *schema.Resource {
type warehousesData struct {
WarehouseNameContains string `json:"warehouse_name_contains,omitempty"`
Ids []string `json:"ids,omitempty" tf:"computed,slice_set"`
Copy link
Contributor

Choose a reason for hiding this comment

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

can you also expose name-to-id mapping?..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what is the best way to represent that? a map[string]string?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes

sql/data_sql_warehouses.go Show resolved Hide resolved
sql/resource_sql_endpoint.go Show resolved Hide resolved
@nkvuong
Copy link
Contributor Author

nkvuong commented Jul 14, 2022

@alexott I would use the warehouses/clusters data sources to search by name, and then use for_each to get attributes for individual compute based on the ids

Copy link
Contributor

@nfx nfx left a comment

Choose a reason for hiding this comment

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

I can merge it if you'll create two follow-up issues:

  • adding integration tests for all these data resources
  • adding name-to-id mapping for warehouses
  • adding lookup of cluster by name

@codecov-commenter
Copy link

Codecov Report

Merging #1460 (b988e49) into master (c852efd) will decrease coverage by 0.01%.
The diff coverage is 96.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1460      +/-   ##
==========================================
- Coverage   90.12%   90.11%   -0.02%     
==========================================
  Files         126      129       +3     
  Lines       10216    10274      +58     
==========================================
+ Hits         9207     9258      +51     
- Misses        642      648       +6     
- Partials      367      368       +1     
Impacted Files Coverage Δ
sql/data_sql_warehouse.go 92.85% <92.85%> (ø)
clusters/data_cluster.go 100.00% <100.00%> (ø)
provider/provider.go 95.07% <100.00%> (+0.10%) ⬆️
sql/data_sql_warehouses.go 100.00% <100.00%> (ø)
sql/resource_sql_endpoint.go 97.43% <100.00%> (ø)
exporter/importables.go 90.54% <0.00%> (-0.69%) ⬇️

@nfx nfx changed the title add data sources for warehouses and clusters Added databricks_sql_warehouses, databricks_sql_warehouse, and databricks_cluster data resources Jul 20, 2022
Copy link
Contributor

@nfx nfx left a comment

Choose a reason for hiding this comment

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

👍

@nfx nfx merged commit 2454acb into master Jul 20, 2022
@nfx nfx deleted the feature/add-data-sources branch July 20, 2022 08:56
@nfx nfx mentioned this pull request Jul 21, 2022
michael-berk pushed a commit to michael-berk/terraform-provider-databricks that referenced this pull request Feb 15, 2023
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.

Data source for a SQL warehouse [FEATURE] data source databricks_cluster
4 participants