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.

Sign up for GitHub

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

Jump to bottom

[Feature] Add data source databricks_notification_destinations #4087

Merged
merged 14 commits into from
Oct 17, 2024
Merged

[Feature] Add data source databricks_notification_destinations #4087

merged 14 commits into from
Oct 17, 2024

Conversation

dgomez04
Copy link
Contributor

@dgomez04 dgomez04 commented Oct 9, 2024 •
edited by alexott
Loading

Changes

Added databricks_notification_destinations data source as requested in #3950 to retrieve notification destinations that are created out-of-band or in other terraform templates.

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

@dgomez04 dgomez04 requested review from a team as code owners October 9, 2024 17:19
@dgomez04 dgomez04 requested review from tanmay-db and removed request for a team October 9, 2024 17:19
@dgomez04 dgomez04 changed the title [Feature] Add datasource databricks_notification_destinations [Feature] Add datasource databricks_notification_destinations Oct 9, 2024
@tanmay-db
Copy link
Contributor

Hi @dgomez04, thanks for the PR, please use plugin framework for the development: https://github.com/databricks/terraform-provider-databricks/blob/main/CONTRIBUTING.md#developing-resources-or-data-sources-using-plugin-framework.

@alexott alexott changed the title [Feature] Add datasource databricks_notification_destinations [Feature] Add data source databricks_notification_destinations Oct 11, 2024
@dgomez04
Copy link
Contributor Author

Hi @tanmay-db! Changes done, please review and let me know if I'm missing anything.

alexott
alexott requested changes Oct 15, 2024
View reviewed changes
Copy link
Contributor

@alexott alexott left a comment

Choose a reason for hiding this comment

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

Overall - quite good, just need to resolve some issues

docs/data-sources/notification_destinations.md Outdated Show resolved Hide resolved
docs/data-sources/notification_destinations.md Outdated Show resolved Hide resolved
...rnal/providers/pluginfw/resources/notificationdestinations/data_notification_destinations.go Outdated Show resolved Hide resolved
...rnal/providers/pluginfw/resources/notificationdestinations/data_notification_destinations.go Outdated Show resolved Hide resolved
...rnal/providers/pluginfw/resources/notificationdestinations/data_notification_destinations.go Outdated Show resolved Hide resolved
...rnal/providers/pluginfw/resources/notificationdestinations/data_notification_destinations.go Outdated Show resolved Hide resolved
...rnal/providers/pluginfw/resources/notificationdestinations/data_notification_destinations.go Outdated Show resolved Hide resolved
...rnal/providers/pluginfw/resources/notificationdestinations/data_notification_destinations.go Outdated Show resolved Hide resolved
...rnal/providers/pluginfw/resources/notificationdestinations/data_notification_destinations.go Outdated Show resolved Hide resolved
...rnal/providers/pluginfw/resources/notificationdestinations/data_notification_destinations.go Outdated Show resolved Hide resolved
@dgomez04
databricks#3950: made requested changes, data source is now case-inse…
c493663 
…nsitive, and returns a list if no matches are found.
@dgomez04 dgomez04 requested a review from alexott October 15, 2024 20:16
alexott
alexott approved these changes Oct 16, 2024
View reviewed changes
Copy link
Contributor

@alexott alexott left a comment

Choose a reason for hiding this comment

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

lgtm with small comment.

let run integration tests for it

docs/data-sources/notification_destinations.md Outdated Show resolved Hide resolved
@alexott
Copy link
Contributor

alexott commented Oct 16, 2024

@dgomez04 you also need to merge the main into this branch and resolve conflict

@alexott alexott added this pull request to the merge queue Oct 17, 2024
Merged via the queue into databricks:main with commit e24c780 Oct 17, 2024
9 checks passed
hectorcast-db added a commit that referenced this pull request Oct 17, 2024
@hectorcast-db
[Release] Release v1.54.0
39adbb7 
### New Features and Improvements

 * Add `databricks_registered_model` data source ([#4033](#4033)).
 * Add data source `databricks_notification_destinations` ([#4087](#4087)).

### Bug Fixes

 * Fix databricks_cluster_pluginframework data source ([#4097](#4097)).
 * Mark unity_catalog_provisioning_state as ReadOnly ([#4116](#4116)).
 * Tolerate invalid keys in `databricks_workspace_conf` ([#4102](#4102)).
 * force send `read_only` in `databricks_external_location` when it's changed ([#4067](#4067)).
 * force send `read_only` in `databricks_storage_credential` when it's changed ([#4083](#4083)).

### Documentation

 * Document `budget_policy_id` in `databricks_pipeline` and `databricks_job` ([#4110](#4110)).
 * Reformat code examples in documentation ([#4081](#4081)).
 * Update documentation for `databricks_model_serving` ([#4115](#4115)).
 * Updates to resource examples ([#4093](#4093)).

### Internal Changes

 * Add maxItem=1 validator for object types in plugin framework schema ([#4094](#4094)).
 * Fix acceptance test for `databricks_registered_model` data source ([#4105](#4105)).
 * Generate Effective Fields ([#4057](#4057)).
 * Generate Effective Fields ([#4112](#4112)).
 * Set SDK used in the useragent in context ([#4092](#4092)).
 * Support adding context in resources and data sources ([#4085](#4085)).
 * Update plugin framework schema to use ListNestedBlocks ([#4040](#4040)).
@hectorcast-db hectorcast-db mentioned this pull request Oct 17, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Oct 18, 2024
@hectorcast-db
[Release] Release v1.54.0 (#4117)
b656822 
### New Features and Improvements

* Add `databricks_registered_model` data source
([#4033](#4033)).
* Add data source `databricks_notification_destinations`
([#4087](#4087)).


### Bug Fixes

* Fix databricks_cluster_pluginframework data source
([#4097](#4097)).
* Mark unity_catalog_provisioning_state as ReadOnly
([#4116](#4116)).
* Tolerate invalid keys in `databricks_workspace_conf`
([#4102](#4102)).
* force send `read_only` in `databricks_external_location` when it's
changed
([#4067](#4067)).
* force send `read_only` in `databricks_storage_credential` when it's
changed
([#4083](#4083)).


### Documentation

* Document `budget_policy_id` in `databricks_pipeline` and
`databricks_job`
([#4110](#4110)).
* Reformat code examples in documentation
([#4081](#4081)).
* Update documentation for `databricks_model_serving`
([#4115](#4115)).
* Updates to resource examples
([#4093](#4093)).


### Internal Changes

* Add maxItem=1 validator for object types in plugin framework schema
([#4094](#4094)).
* Fix acceptance test for `databricks_registered_model` data source
([#4105](#4105)).
* Generate Effective Fields
([#4057](#4057)).
* Generate Effective Fields
([#4112](#4112)).
* Set SDK used in the useragent in context
([#4092](#4092)).
* Support adding context in resources and data sources
([#4085](#4085)).
* Update plugin framework schema to use ListNestedBlocks
([#4040](#4040)).
@dgomez04 dgomez04 mentioned this pull request Oct 18, 2024
Closed
@dgomez04 dgomez04 deleted the feature/3950-databricks_notification_destination branch October 22, 2024 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexott alexott alexott approved these changes

@tanmay-db tanmay-db Awaiting requested review from tanmay-db tanmay-db is a code owner automatically assigned from databricks/eng-plat-auto-exp-reviewers

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dgomez04 @tanmay-db @alexott