Skip to content

Commit 3a183ce

Browse files
[Fix] Add support for all error details types and fix potential unmarshalling error (#912)
## What changes are proposed in this pull request? This PR refactors how `DatabricksError` processes error details to fully support the Databricks Error specification. In particular, it provides a clear mechanism to access known error details types while still enabling users to access unknown error details. The core of the PR is the addition of a new `ErrorDetails` type which operates as the union of all error details type. It implements the fact that an error response should only contain at most one instance of each type. The code is purposely structured to hide implementation details by separating the exported error details types from their unmarshalling logic. This arguably leads to code that might be more complicated that it ought to be. We could consider further simplifying it in the future (e.g. by relying on the corresponding proto unmarshallers). ## How is this tested? 100% unit tests coverage of new code + slight refactor of the test suite.
1 parent a1185d2 commit 3a183ce

File tree

6 files changed

+756
-170
lines changed

6 files changed

+756
-170
lines changed

NEXT_CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
### Documentation
1010

1111
### Internal Changes
12+
13+
* Refactor `DatabricksError` to expose different types of error details ([#912](https://github.com/databricks/databricks-sdk-py/pull/912)).
1214
* Update Jobs ListJobs API to support paginated responses ([#896](https://github.com/databricks/databricks-sdk-py/pull/896))
1315
* Update Jobs ListRuns API to support paginated responses ([#890](https://github.com/databricks/databricks-sdk-py/pull/890))
1416
* Introduce automated tagging ([#888](https://github.com/databricks/databricks-sdk-py/pull/888))

databricks/sdk/errors/base.py

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@
55

66
import requests
77

8+
from . import details as errdetails
89

10+
11+
# Deprecated.
912
class ErrorDetail:
1013
def __init__(
1114
self,
@@ -22,17 +25,18 @@ def __init__(
2225

2326
@classmethod
2427
def from_dict(cls, d: Dict[str, any]) -> "ErrorDetail":
25-
if "@type" in d:
26-
d["type"] = d["@type"]
27-
return cls(**d)
28+
# Key "@type" is not a valid keyword argument name in Python. Rename
29+
# it to "type" to avoid conflicts.
30+
safe_args = {}
31+
for k, v in d.items():
32+
safe_args[k if k != "@type" else "type"] = v
33+
34+
return cls(**safe_args)
2835

2936

3037
class DatabricksError(IOError):
3138
"""Generic error from Databricks REST API"""
3239

33-
# Known ErrorDetail types
34-
_error_info_type = "type.googleapis.com/google.rpc.ErrorInfo"
35-
3640
def __init__(
3741
self,
3842
message: str = None,
@@ -54,11 +58,11 @@ def __init__(
5458
:param status: [Deprecated]
5559
:param scimType: [Deprecated]
5660
:param error: [Deprecated]
57-
:param retry_after_secs:
61+
:param retry_after_secs: [Deprecated]
5862
:param details:
5963
:param kwargs:
6064
"""
61-
# SCIM-specific parameters are deprecated
65+
# SCIM-specific parameters are deprecated.
6266
if detail:
6367
warnings.warn(
6468
"The 'detail' parameter of DatabricksError is deprecated and will be removed in a future version."
@@ -72,12 +76,18 @@ def __init__(
7276
"The 'status' parameter of DatabricksError is deprecated and will be removed in a future version."
7377
)
7478

75-
# API 1.2-specific parameters are deprecated
79+
# API 1.2-specific parameters are deprecated.
7680
if error:
7781
warnings.warn(
7882
"The 'error' parameter of DatabricksError is deprecated and will be removed in a future version."
7983
)
8084

85+
# Retry-after is deprecated.
86+
if retry_after_secs:
87+
warnings.warn(
88+
"The 'retry_after_secs' parameter of DatabricksError is deprecated and will be removed in a future version."
89+
)
90+
8191
if detail:
8292
# Handle SCIM error message details
8393
# @see https://tools.ietf.org/html/rfc7644#section-3.7.3
@@ -88,20 +98,32 @@ def __init__(
8898
# add more context from SCIM responses
8999
message = f"{scimType} {message}".strip(" ")
90100
error_code = f"SCIM_{status}"
101+
91102
super().__init__(message if message else error)
92103
self.error_code = error_code
93104
self.retry_after_secs = retry_after_secs
94-
self.details = [ErrorDetail.from_dict(detail) for detail in details] if details else []
105+
self._error_details = errdetails.parse_error_details(details)
95106
self.kwargs = kwargs
96107

108+
# Deprecated.
109+
self.details = []
110+
if details:
111+
for d in details:
112+
if not isinstance(d, dict):
113+
continue
114+
self.details.append(ErrorDetail.from_dict(d))
115+
97116
def get_error_info(self) -> List[ErrorDetail]:
98-
return self._get_details_by_type(DatabricksError._error_info_type)
117+
return self._get_details_by_type(errdetails._ERROR_INFO_TYPE)
99118

100119
def _get_details_by_type(self, error_type) -> List[ErrorDetail]:
101-
if self.details == None:
120+
if self.details is None:
102121
return []
103122
return [detail for detail in self.details if detail.type == error_type]
104123

124+
def get_error_details(self) -> errdetails.ErrorDetails:
125+
return self._error_details
126+
105127

106128
@dataclass
107129
class _ErrorOverride:

0 commit comments

Comments
 (0)