Skip to content

Introduce more specific exceptions, like errors.Is(err, databricks.ErrNotFound), errors.Is(err, databricks.ErrAlreadyExists), errors.Is(err, databricks.ErrBadRequest), errors.Is(err, databricks.ErrPermissionDenied), errors.Is(err, databricks.ErrInternal), and others#682

Merged
nfx merged 6 commits intomainfrom
ergonomics/errors
Nov 13, 2023

Conversation

@nfx
Copy link
Copy Markdown
Contributor

@nfx nfx commented Nov 7, 2023

Changes

This PR enables ergonomic error handling for all Databricks API Errors, following the same principles as databricks/databricks-sdk-py#376

Tests

  • make test passing
  • make fmt applied
  • relevant integration tests applied

…rrNotFound)`, `errors.Is(err, databricks.ErrAlreadyExists)`, `errors.Is(err, databricks.ErrBadRequest)`, `errors.Is(err, databricks.ErrPermissionDenied)`, `errors.Is(err, databricks.ErrInternal)`, and others

This PR enables ergonomic error handling for all Databricks API Errors, following the same principles as databricks/databricks-sdk-py#376
@nfx nfx requested review from mgyucht and pietern November 7, 2023 15:57
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Files Coverage Δ
apierr/unwrap.go 83.33% <83.33%> (ø)

... and 1 file with indirect coverage changes

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Contributor

@mgyucht mgyucht left a comment

Choose a reason for hiding this comment

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

This is terrific, thank you very much for going along the autogenerated route. I think this is a good outcome for the project as far as maintainability goes. My only preference is for now not to include any other errors in this hierarchy. If clients want to wrap these errors explicitly, they should be able to do so.

nfx added a commit to databricks/databricks-sdk-java that referenced this pull request Nov 10, 2023
… `BadRequest`, `PermissionDenied`, `InternalError`, and others

See implementations in other SDKs:

- Go: databricks/databricks-sdk-go#682
- Python: databricks/databricks-sdk-py#376
@nfx nfx requested a review from mgyucht November 10, 2023 16:24
@nfx nfx enabled auto-merge November 10, 2023 16:39
@nfx nfx added this pull request to the merge queue Nov 13, 2023
Merged via the queue into main with commit 5369aac Nov 13, 2023
@nfx nfx deleted the ergonomics/errors branch November 13, 2023 12:49
mgyucht added a commit that referenced this pull request Nov 29, 2023
Major changes:

* There has been a major overhaul of error handling. Users can now compare errors in API responses to the well-known error responses defined in the `apierr` package and reexported in the `databricks` package. Users can check whether a specific error was returned, for example `errors.Is(err, databricks.ErrResourceAlreadyExists)`, rather than converting the error to `*APIError` to check the status code and error code. This change is backwards-compatible; users do not need to modify existing error-handling code when upgrading the SDK. See [#682](#682) and [#703](#703) for the changes and https://github.com/databricks/databricks-sdk-go/blob/main/error_alias.go for the full set of errors.

Bug fixes:

* Handle "no configuration file found at" error during databricks-cli authentication ([#707](#707)).
* Introduce `DatabricksEnvironment` and fix Azure MSI auth from ACR, where IMDS doesn't give host environment information ([#700](#700)).
* Fix SCIM Pagination default parameters in the Go SDK ([#717](#717)).

Other changes:

* Update `slog` example with the correct interface ([#694](#694)).
* Fixed typo in error message for unknown azure environment ([#701](#701)).
* Allow injection of HTTP transport to enable HTTP replayer pattern ([#697](#697)).
* Decouple HTTP retries and error mapping mechanics from `DatabricksClient` into `httpclient.ApiClient` ([#699](#699), [#702](#702), [#712](#712)).
* Port `qa.HTTPFixtures` to faster transport-level stubs ([#708](#708)).

API Changes:

 * Removed `EnableOptimization` method for [w.Metastores](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/catalog#MetastoresAPI) workspace-level service.
 * Added `PipelineId` field for [catalog.TableInfo](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/catalog#TableInfo).
 * Added `EnablePredictiveOptimization` field for [catalog.UpdateCatalog](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/catalog#UpdateCatalog) and [catalog.UpdateSchema](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/catalog#UpdateSchema).
 * Removed [catalog.UpdatePredictiveOptimization](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/catalog#UpdatePredictiveOptimization) and [catalog.UpdatePredictiveOptimizationResponse](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/catalog#UpdatePredictiveOptimizationResponse).
 * Added `Description` field for [jobs.CreateJob](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/jobs#CreateJob) and [jobs.JobSettings](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/jobs#JobSettings).
 * Added `ListNetworkConnectivityConfigurations` and `ListPrivateEndpointRules` method for [a.NetworkConnectivity](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/settings#NetworkConnectivityAPI) account-level service.
 * Added [settings.ListNccAzurePrivateEndpointRulesResponse](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/settings#ListNccAzurePrivateEndpointRulesResponse), [settings.ListNetworkConnectivityConfigurationsRequest](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/settings#ListNetworkConnectivityConfigurationsRequest), [settings.ListNetworkConnectivityConfigurationsResponse](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/settings#ListNetworkConnectivityConfigurationsResponse), and [settings.ListPrivateEndpointRulesRequest](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/settings#ListPrivateEndpointRulesRequest).
 * Added `StringSharedAs` field for [sharing.SharedDataObject](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/sharing#SharedDataObject).

Internal changes:

* Added `contains` method in OpenAPI Generator ([#690](#690)).
* Skip recipients tests in Azure ([#692](#692)).
* Allow Files API tests to run in UC environments ([#695](#695)).
* More cleanup in Unity Catalog integration test ([#719](#719)).

OpenAPI SHA: 22f09783eb8a84d52026f856be3b2068f9498db3, Date: 2023-11-23
Dependency updates:

 * Bump golang.org/x/oauth2 from 0.13.0 to 0.14.0 ([#689](#689)).
 * Bump google.golang.org/api from 0.150.0 to 0.151.0 ([#698](#698)).
 * Bump the OpenAPI Spec ([#706](#706)).
 * Bump golang.org/x/oauth2 from 0.14.0 to 0.15.0 ([#715](#715)).
 * Bump golang.org/x/time from 0.4.0 to 0.5.0 ([#714](#714)).
 * Bump google.golang.org/api from 0.151.0 to 0.152.0 ([#716](#716)).
@mgyucht mgyucht mentioned this pull request Nov 29, 2023
github-merge-queue bot pushed a commit that referenced this pull request Nov 29, 2023
Major changes:

* There has been a major overhaul of error handling. Users can now
compare errors in API responses to the well-known error responses
defined in the `apierr` package and reexported in the `databricks`
package. Users can check whether a specific error was returned, for
example `errors.Is(err, databricks.ErrResourceAlreadyExists)`, rather
than converting the error to `*APIError` to check the status code and
error code. This change is backwards-compatible; users do not need to
modify existing error-handling code when upgrading the SDK. See
[#682](#682) and
[#703](#703) for the
changes and
https://github.com/databricks/databricks-sdk-go/blob/main/error_alias.go
for the full set of errors.

Bug fixes:

* Handle "no configuration file found at" error during databricks-cli
authentication
([#707](#707)).
* Introduce `DatabricksEnvironment` and fix Azure MSI auth from ACR,
where IMDS doesn't give host environment information
([#700](#700)).
* Fix SCIM Pagination default parameters in the Go SDK
([#717](#717)).

Other changes:

* Update `slog` example with the correct interface
([#694](#694)).
* Fixed typo in error message for unknown azure environment
([#701](#701)).
* Allow injection of HTTP transport to enable HTTP replayer pattern
([#697](#697)).
* Decouple HTTP retries and error mapping mechanics from
`DatabricksClient` into `httpclient.ApiClient`
([#699](#699),
[#702](#702),
[#712](#712)).
* Port `qa.HTTPFixtures` to faster transport-level stubs
([#708](#708)).

API Changes:

* Removed `EnableOptimization` method for
[w.Metastores](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/catalog#MetastoresAPI)
workspace-level service.
* Added `PipelineId` field for
[catalog.TableInfo](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/catalog#TableInfo).
* Added `EnablePredictiveOptimization` field for
[catalog.UpdateCatalog](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/catalog#UpdateCatalog)
and
[catalog.UpdateSchema](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/catalog#UpdateSchema).
* Removed
[catalog.UpdatePredictiveOptimization](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/catalog#UpdatePredictiveOptimization)
and
[catalog.UpdatePredictiveOptimizationResponse](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/catalog#UpdatePredictiveOptimizationResponse).
* Added `Description` field for
[jobs.CreateJob](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/jobs#CreateJob)
and
[jobs.JobSettings](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/jobs#JobSettings).
* Added `ListNetworkConnectivityConfigurations` and
`ListPrivateEndpointRules` method for
[a.NetworkConnectivity](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/settings#NetworkConnectivityAPI)
account-level service.
* Added
[settings.ListNccAzurePrivateEndpointRulesResponse](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/settings#ListNccAzurePrivateEndpointRulesResponse),
[settings.ListNetworkConnectivityConfigurationsRequest](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/settings#ListNetworkConnectivityConfigurationsRequest),
[settings.ListNetworkConnectivityConfigurationsResponse](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/settings#ListNetworkConnectivityConfigurationsResponse),
and
[settings.ListPrivateEndpointRulesRequest](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/settings#ListPrivateEndpointRulesRequest).
* Added `StringSharedAs` field for
[sharing.SharedDataObject](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/sharing#SharedDataObject).

Internal changes:

* Added `contains` method in OpenAPI Generator
([#690](#690)).
* Skip recipients tests in Azure
([#692](#692)).
* Allow Files API tests to run in UC environments
([#695](#695)).
* More cleanup in Unity Catalog integration test
([#719](#719)).

OpenAPI SHA: 22f09783eb8a84d52026f856be3b2068f9498db3, Date: 2023-11-23
Dependency updates:

* Bump golang.org/x/oauth2 from 0.13.0 to 0.14.0
([#689](#689)).
* Bump google.golang.org/api from 0.150.0 to 0.151.0
([#698](#698)).
* Bump the OpenAPI Spec
([#706](#706)).
* Bump golang.org/x/oauth2 from 0.14.0 to 0.15.0
([#715](#715)).
* Bump golang.org/x/time from 0.4.0 to 0.5.0
([#714](#714)).
* Bump google.golang.org/api from 0.151.0 to 0.152.0
([#716](#716)).
github-merge-queue bot pushed a commit to databricks/databricks-sdk-java that referenced this pull request Apr 4, 2024
… `BadRequest`, `PermissionDenied`, `InternalError`, and others (#185)

See implementations in other SDKs:

- Go: databricks/databricks-sdk-go#682
- Python: databricks/databricks-sdk-py#376

---------

Co-authored-by: Miles Yucht <[email protected]>
Co-authored-by: Tanmay Rustagi <[email protected]>
vikrantpuppala pushed a commit to vikrantpuppala/databricks-sdk-java that referenced this pull request Apr 23, 2024
… `BadRequest`, `PermissionDenied`, `InternalError`, and others (databricks#185)

See implementations in other SDKs:

- Go: databricks/databricks-sdk-go#682
- Python: databricks/databricks-sdk-py#376

---------

Co-authored-by: Miles Yucht <[email protected]>
Co-authored-by: Tanmay Rustagi <[email protected]>
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.

3 participants