terraform-provider-databricks icon indicating copy to clipboard operation
terraform-provider-databricks copied to clipboard

Adds `databricks_volume` as data source

Open karolusz opened this issue 1 year ago • 7 comments

Changes

Adds databricks_volume data source.

The data source takes the volume's full_name or catalog_name, schema_name, name as user input.

~~I tried to use the schema customization from common.customizable_schema.go when defining data source's parameters. The following changes were made in common/resource.go as none of the function wrapping around genericDatabricksData and the genericDatabricksData itself was not allowing to pass a customization function:~~

~~- genericDatabricksData now takes a schema customization function for customizing otherFields which is overlaid on top SDK schema.~~ ~~- a new function WorkspaceDataWithCustomParams was added that wraps genericDatabricksData for cases where a schema customization function is desired.~~ ~~- WorkspaceData, WorkspaceDataWithParams, AccountData, AccountDataWithParams were adjusted to pass NoCustomize to the updated genericDatabricksData~~

The above changes were taken out because of #3207

Tests

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

karolusz avatar Feb 02 '24 21:02 karolusz

Codecov Report

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

Project coverage is 82.56%. Comparing base (6cfcb2b) to head (5d21bc8). Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3211      +/-   ##
==========================================
+ Coverage   82.55%   82.56%   +0.01%     
==========================================
  Files         186      188       +2     
  Lines       19131    19177      +46     
==========================================
+ Hits        15794    15834      +40     
- Misses       2411     2415       +4     
- Partials      926      928       +2     
Files Coverage Δ
common/resource.go 74.64% <100.00%> (ø)
provider/provider.go 93.93% <100.00%> (+0.05%) :arrow_up:
catalog/data_volume.go 90.00% <90.00%> (ø)

... and 2 files with indirect coverage changes

codecov-commenter avatar Feb 03 '24 10:02 codecov-commenter

Added docs, tests and resolved review comments.

karolusz avatar Feb 04 '24 23:02 karolusz

Formatting fixed.

karolusz avatar Feb 05 '24 08:02 karolusz

Small refactor to take advantage of #3207 and avoid having to add new functions in common/resource.go

Tests and docs added.

karolusz avatar Feb 14 '24 21:02 karolusz

@alexott the requested changes were addressed. Please let me know if you see anything else outstanding

karolusz avatar Feb 19 '24 09:02 karolusz

Update: Merged the latest changes from main and made adjustments for the latest version of GO SDK.

karolusz avatar Apr 04 '24 21:04 karolusz

@nkvuong @mgyucht I think that we need to discuss the unified approach to the data sources implementation - the metastore, catalog, and table sources are exposing data as a nested object metastore_info, catalog_info, etc., while storage credential, external location and volume data sources are putting the data as flat structure...

alexott avatar May 15 '24 05:05 alexott

We discussed this in the slack thread (mentioning here for visibility) but since most of the data sources use nested structure, I think we should use that here as well for consistency.

The changes were made to comply with the approach of having data sources as nested structures.

karolusz avatar Jul 02 '24 04:07 karolusz

Integration test passed

alexott avatar Jul 02 '24 10:07 alexott

thank you for contribution @karolusz !

alexott avatar Jul 03 '24 07:07 alexott