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

[Exporter] Improve exporting of databricks_model_serving #3821

Merged
merged 1 commit into from
Jul 26, 2024
Merged

[Exporter] Improve exporting of databricks_model_serving #3821

merged 1 commit into from
Jul 26, 2024

Conversation

alexott
Copy link
Contributor

@alexott alexott commented Jul 25, 2024

Changes

Changes include:

  • emitting references to UC models
  • emitting references to UC catalog and schema for auto-capture
  • emitting secrets from environment variables and external models configuration
  • generation of some required fields
  • expanding test coverage

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

@alexott
[Exporter] Improve exporting of databricks_model_serving
ebaf0ec 
Changes include:

- emitting references to UC models
- emitting references to UC catalog and schema for auto-capture
- emitting secrets from environment variables and external models configuration
- generation of some required fields
- expanding test coverage
@alexott alexott requested review from a team as code owners July 25, 2024 10:54
@alexott alexott requested review from tanmay-db and removed request for a team July 25, 2024 10:54
@alexott alexott added the exporter TF configuration generator label Jul 25, 2024
tanmay-db
tanmay-db approved these changes Jul 26, 2024
View reviewed changes
exporter/importables.go
if res := servedEntityFieldExtractionRegex.FindStringSubmatch(pathString); res != nil {
field := res[2]
log.Printf("[DEBUG] ShouldOmitField: extracted field from %s: '%s'", pathString, field)
switch field {
Copy link
Contributor

Choose a reason for hiding this comment

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

should we have a default case here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Default case could be also complex, so it's better to pass through and allow the last line of the function (call to the defaultShouldOmitFieldFunc) handle it

exporter/importables.go
return defaultShouldOmitFieldFunc(ic, pathString, as, d)
},
ShouldGenerateField: func(ic *importContext, pathString string, as *schema.Schema, d *schema.ResourceData) bool {
// We need to generate some fields even if they have zero value...
Copy link
Contributor

Choose a reason for hiding this comment

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

For my understanding, why do we want to generate these fields? Also, are there other fields for which we would like to do this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the exporter, when code is generated, we're checking if the field has omitempty and zero value, and skipping its generation. But in the current case, the field is marked as omitempty in the schema, and has a zero value, but we still need to generate it (similar case is for serverless option in SQL warehouses).

@alexott alexott added this pull request to the merge queue Jul 26, 2024
Merged via the queue into main with commit 1df8285 Jul 26, 2024
6 checks passed
@alexott alexott deleted the exporter-model-serving-improvements branch July 26, 2024 13:45
mgyucht added a commit that referenced this pull request Aug 14, 2024
@mgyucht
[Release] Release v1.49.2
f235ec9 
### New Features and Improvements

 * Added support for `cloudflare_api_token` in `databricks_storage_credential` resource ([#3835](#3835)).
 * Add `active` attribute to `databricks_user` data source ([#3733](#3733)).
 * Add `workspace_path` attribute to `databricks_notebook` resource and data source ([#3885](#3885)).
 * Mark attributes as sensitive in `databricks_mlflow_webhook` ([#3825](#3825)).
 * Added notification destination resource ([#3820](#3820)).

### Bug Fixes

 * Automatically assign `IS_OWNER` permission to sql warehouse if not specified ([#3829](#3829)).
 * Corrected kms arn format in `data_aws_unity_catalog_policy` ([#3823](#3823)).
 * Fix crash when destroying `databricks_compliance_security_profile_workspace_setting` ([#3883](#3883)).
 * Fixed read method of `databricks_entitlements` resource ([#3858](#3858)).
 * Retry cluster update on "INVALID_STATE" ([#3890](#3890)).
 * Save Pipeline resource to state in addition to spec ([#3869](#3869)).
 * Tolerate `databricks_workspace_conf` deletion failures ([#3737](#3737)).
 * Update Go SDK ([#3826](#3826)).
 * cluster key update for `databricks_sql_table` should not force new ([#3824](#3824)).
 * reading `databricks_metastore_assignment` when importing resource ([#3827](#3827)).

### Documentation

 * Add troubleshooting instructions for `databricks OAuth is not supported for this host` error ([#3815](#3815)).
 * Clarify setting of permissions for workspace objects ([#3884](#3884)).
 * Document missing task attributes in `databricks_job` resource ([#3817](#3817)).
 * Fixed documentation for `databricks_schemas` data source and `databricks_metastore_assignment` resource ([#3851](#3851)).
 * clarified `spot_bid_max_price` option for `databricks_cluster` ([#3830](#3830)).
 * marked `databricks_sql_dashboard` as legacy ([#3836](#3836)).

### Internal Changes

 * Refactor exporter: split huge files into smaller ones ([#3870](#3870)).
 * Refactored `client.ClientForHost` to use Go SDK method ([#3735](#3735)).
 * Revert "Rewriting DLT pipelines using SDK" ([#3838](#3838)).
 * Rewrite DLT pipelines using SDK ([#3839](#3839)).
 * Rewriting DLT pipelines using SDK ([#3792](#3792)).
 * Update Go SDK ([#3808](#3808)).
 * refactored `databricks_mws_permission_assignment` to Go SDK ([#3831](#3831)).

### Dependency Updates

 * Bump databricks-sdk-go to 0.44.0 ([#3896](#3896)).
 * Bump github.com/zclconf/go-cty from 1.14.4 to 1.15.0 ([#3775](#3775)).

### Exporter

 * Add retry on "Operation timed out" error ([#3897](#3897)).
 * Add support for Vector Search assets ([#3828](#3828)).
 * Add support for `databricks_notification_destination` ([#3861](#3861)).
 * Add support for `databricks_online_table` ([#3816](#3816)).
 * Don't export model serving endpoints with foundational models ([#3845](#3845)).
 * Fix generation of `autotermination_minutes = 0` ([#3881](#3881)).
 * Generate `databricks_workspace_binding` instead of legacy `databricks_catalog_workspace_binding` ([#3812](#3812)).
 * Ignore DLT pipelines deployed via DABs ([#3857](#3857)).
 * Improve exporting of `databricks_model_serving` ([#3821](#3821)).
 * Refactoring: remove legacy code ([#3864](#3864)).
@mgyucht mgyucht mentioned this pull request Aug 14, 2024
Merged
mgyucht added a commit that referenced this pull request Aug 14, 2024
@mgyucht
[Release] Release v1.50.0
05af14d 
### New Features and Improvements

 * Added `databricks_notification_destination` resource ([#3820](#3820)).
 * Added support for `cloudflare_api_token` in `databricks_storage_credential` resource ([#3835](#3835)).
 * Add `active` attribute to `databricks_user` data source ([#3733](#3733)).
 * Add `workspace_path` attribute to `databricks_notebook` resource and data source ([#3885](#3885)).
 * Mark attributes as sensitive in `databricks_mlflow_webhook` ([#3825](#3825)).

### Bug Fixes

 * Automatically assign `IS_OWNER` permission to sql warehouse if not specified ([#3829](#3829)).
 * Corrected kms arn format in `data_aws_unity_catalog_policy` ([#3823](#3823)).
 * Fix crash when destroying `databricks_compliance_security_profile_workspace_setting` ([#3883](#3883)).
 * Fixed read method of `databricks_entitlements` resource ([#3858](#3858)).
 * Retry cluster update on "INVALID_STATE" ([#3890](#3890)).
 * Save Pipeline resource to state in addition to spec ([#3869](#3869)).
 * Tolerate `databricks_workspace_conf` deletion failures ([#3737](#3737)).
 * Update Go SDK ([#3826](#3826)).
 * cluster key update for `databricks_sql_table` should not force new ([#3824](#3824)).
 * reading `databricks_metastore_assignment` when importing resource ([#3827](#3827)).

### Documentation

 * Add troubleshooting instructions for `databricks OAuth is not supported for this host` error ([#3815](#3815)).
 * Clarify setting of permissions for workspace objects ([#3884](#3884)).
 * Document missing task attributes in `databricks_job` resource ([#3817](#3817)).
 * Fixed documentation for `databricks_schemas` data source and `databricks_metastore_assignment` resource ([#3851](#3851)).
 * clarified `spot_bid_max_price` option for `databricks_cluster` ([#3830](#3830)).
 * marked `databricks_sql_dashboard` as legacy ([#3836](#3836)).

### Internal Changes

 * Refactor exporter: split huge files into smaller ones ([#3870](#3870)).
 * Refactored `client.ClientForHost` to use Go SDK method ([#3735](#3735)).
 * Revert "Rewriting DLT pipelines using SDK" ([#3838](#3838)).
 * Rewrite DLT pipelines using SDK ([#3839](#3839)).
 * Rewriting DLT pipelines using SDK ([#3792](#3792)).
 * Update Go SDK ([#3808](#3808)).
 * refactored `databricks_mws_permission_assignment` to Go SDK ([#3831](#3831)).

### Dependency Updates

 * Bump databricks-sdk-go to 0.44.0 ([#3896](#3896)).
 * Bump github.com/zclconf/go-cty from 1.14.4 to 1.15.0 ([#3775](#3775)).

### Exporter

 * Add retry on "Operation timed out" error ([#3897](#3897)).
 * Add support for Vector Search assets ([#3828](#3828)).
 * Add support for `databricks_notification_destination` ([#3861](#3861)).
 * Add support for `databricks_online_table` ([#3816](#3816)).
 * Don't export model serving endpoints with foundational models ([#3845](#3845)).
 * Fix generation of `autotermination_minutes = 0` ([#3881](#3881)).
 * Generate `databricks_workspace_binding` instead of legacy `databricks_catalog_workspace_binding` ([#3812](#3812)).
 * Ignore DLT pipelines deployed via DABs ([#3857](#3857)).
 * Improve exporting of `databricks_model_serving` ([#3821](#3821)).
 * Refactoring: remove legacy code ([#3864](#3864)).
github-merge-queue bot pushed a commit that referenced this pull request Aug 15, 2024
@mgyucht
[Release] Release v1.50.0 (#3900)
79b9377 
### New Features and Improvements

* Added `databricks_notification_destination` resource
([#3820](#3820)).
* Added support for `cloudflare_api_token` in
`databricks_storage_credential` resource
([#3835](#3835)).
* Add `active` attribute to `databricks_user` data source
([#3733](#3733)).
* Add `workspace_path` attribute to `databricks_notebook` resource and
data source
([#3885](#3885)).
* Mark attributes as sensitive in `databricks_mlflow_webhook`
([#3825](#3825)).


### Bug Fixes

* Automatically assign `IS_OWNER` permission to sql warehouse if not
specified
([#3829](#3829)).
* Corrected kms arn format in `data_aws_unity_catalog_policy`
([#3823](#3823)).
* Fix crash when destroying
`databricks_compliance_security_profile_workspace_setting`
([#3883](#3883)).
* Fixed read method of `databricks_entitlements` resource
([#3858](#3858)).
* Retry cluster update on "INVALID_STATE"
([#3890](#3890)).
* Save Pipeline resource to state in addition to spec
([#3869](#3869)).
* Tolerate `databricks_workspace_conf` deletion failures
([#3737](#3737)).
* Update Go SDK
([#3826](#3826)).
* cluster key update for `databricks_sql_table` should not force new
([#3824](#3824)).
* reading `databricks_metastore_assignment` when importing resource
([#3827](#3827)).


### Documentation

* Add troubleshooting instructions for `databricks OAuth is not
supported for this host` error
([#3815](#3815)).
* Clarify setting of permissions for workspace objects
([#3884](#3884)).
* Document missing task attributes in `databricks_job` resource
([#3817](#3817)).
* Fixed documentation for `databricks_schemas` data source and
`databricks_metastore_assignment` resource
([#3851](#3851)).
* clarified `spot_bid_max_price` option for `databricks_cluster`
([#3830](#3830)).
* marked `databricks_sql_dashboard` as legacy
([#3836](#3836)).


### Internal Changes

* Refactor exporter: split huge files into smaller ones
([#3870](#3870)).
* Refactored `client.ClientForHost` to use Go SDK method
([#3735](#3735)).
* Revert "Rewriting DLT pipelines using SDK"
([#3838](#3838)).
* Rewrite DLT pipelines using SDK
([#3839](#3839)).
* Rewriting DLT pipelines using SDK
([#3792](#3792)).
* Update Go SDK
([#3808](#3808)).
* refactored `databricks_mws_permission_assignment` to Go SDK
([#3831](#3831)).


### Dependency Updates

* Bump databricks-sdk-go to 0.44.0
([#3896](#3896)).
* Bump github.com/zclconf/go-cty from 1.14.4 to 1.15.0
([#3775](#3775)).


### Exporter

* Add retry on "Operation timed out" error
([#3897](#3897)).
* Add support for Vector Search assets
([#3828](#3828)).
* Add support for `databricks_notification_destination`
([#3861](#3861)).
* Add support for `databricks_online_table`
([#3816](#3816)).
* Don't export model serving endpoints with foundational models
([#3845](#3845)).
* Fix generation of `autotermination_minutes = 0`
([#3881](#3881)).
* Generate `databricks_workspace_binding` instead of legacy
`databricks_catalog_workspace_binding`
([#3812](#3812)).
* Ignore DLT pipelines deployed via DABs
([#3857](#3857)).
* Improve exporting of `databricks_model_serving`
([#3821](#3821)).
* Refactoring: remove legacy code
([#3864](#3864)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tanmay-db tanmay-db tanmay-db approved these changes

Assignees
No one assigned
Labels
exporter TF configuration generator
Projects
None yet
Milestone
No milestone
Development

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