Skip to content

Commit 93a622d

Browse files
nfxmgyucht
nfx
and
mgyucht
authored
Introduce more specific exceptions, like NotFound, AlreadyExists, BadRequest, PermissionDenied, InternalError, and others (#376)
Improve the ergonomics of SDK, where instead of `except DatabricksError as err: if err.error_code != 'NOT_FOUND': raise err else: do_stuff()` we could do `except NotFound: do_stuff()`, like in [this example](https://github.com/databrickslabs/ucx/blob/main/src/databricks/labs/ucx/workspace_access/generic.py#L71-L84). Additionally, it'll make it easier to read stack traces, as they will contain specific exception class name. Examples of unclear stack traces are: databrickslabs/ucx#359, databrickslabs/ucx#353, databrickslabs/ucx#347, # First principles - ~~do not override `builtins.NotImplemented` for `NOT_IMPLEMENTED` error code~~ - assume that platform error_code/HTTP status code mapping is not perfect and in the state of transition - we do represent reasonable subset of error codes as specific exceptions - it's still possible to access `error_code` from specific exceptions like `NotFound` or `AlreadyExists`. # Proposal - have hierarchical exceptions, also inheriting from Python's built-in exceptions - more specific error codes override more generic HTTP status codes - more generic HTTP status codes matched after more specific error codes, where there's a default exception class per HTTP status code, and we do rely on Databricks platform exception mapper to do the right thing. - have backward-compatible error creation for cases like using older versions of the SDK on the way never releases of the platform. ![image](https://github.com/databricks/databricks-sdk-py/assets/259697/a4519f76-0778-468c-9bf5-6133984b5af7) ### Naming conflict resolution We have four sources of naming and this is a proposed order of naming conflict resolution: 1. Databricks `error_code`, that is surfaced in our API documentation, known by Databricks users 2. HTTP Status codes, known by some developers 3. Python builtin exceptions, known by some developers 4. grpc error codes https://github.com/googleapis/googleapis/blob/master/google/rpc/code.proto#L38-L185, know by some developers --------- Co-authored-by: Miles Yucht <[email protected]>
1 parent a862adc commit 93a622d

File tree

10 files changed

+277
-72
lines changed

10 files changed

+277
-72
lines changed

‎.codegen.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
".codegen/service.py.tmpl": "databricks/sdk/service/{{.Name}}.py"
55
},
66
"batch": {
7-
".codegen/__init__.py.tmpl": "databricks/sdk/__init__.py"
7+
".codegen/__init__.py.tmpl": "databricks/sdk/__init__.py",
8+
".codegen/error_mapping.py.tmpl": "databricks/sdk/errors/mapping.py"
89
},
910
"samples": {
1011
".codegen/example.py.tmpl": "examples/{{.Service.SnakeName}}/{{.Method.SnakeName}}_{{.SnakeName}}.py"

‎.codegen/error_mapping.py.tmpl‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# Code generated from OpenAPI specs by Databricks SDK Generator. DO NOT EDIT.
2+
3+
from .base import DatabricksError
4+
5+
{{range .ExceptionTypes}}
6+
class {{.PascalName}}({{if .Inherit -}}
7+
{{.Inherit.PascalName}}
8+
{{- else -}}
9+
DatabricksError
10+
{{- end -}}):
11+
"""{{.Comment " " 100 | trimSuffix "\"" }}"""
12+
{{end}}
13+
14+
STATUS_CODE_MAPPING = { {{range .ErrorStatusCodeMapping}}
15+
{{.StatusCode}}: {{.PascalName}},{{- end}}
16+
}
17+
18+
ERROR_CODE_MAPPING = { {{range .ErrorCodeMapping}}
19+
'{{.ErrorCode}}': {{.PascalName}},{{- end}}
20+
}

‎databricks/sdk/core.py‎

Lines changed: 2 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
from .azure import (ARM_DATABRICKS_RESOURCE_ID, ENVIRONMENTS, AzureEnvironment,
2626
add_sp_management_token, add_workspace_id_header)
27+
from .errors import DatabricksError, error_mapper
2728
from .oauth import (ClientCredentials, OAuthClient, OidcEndpoints, Refreshable,
2829
Token, TokenCache, TokenSource)
2930
from .retries import retried
@@ -963,70 +964,6 @@ def copy(self):
963964
return cpy
964965

965966

966-
class ErrorDetail:
967-
968-
def __init__(self,
969-
type: str = None,
970-
reason: str = None,
971-
domain: str = None,
972-
metadata: dict = None,
973-
**kwargs):
974-
self.type = type
975-
self.reason = reason
976-
self.domain = domain
977-
self.metadata = metadata
978-
979-
@classmethod
980-
def from_dict(cls, d: Dict[str, any]) -> 'ErrorDetail':
981-
if '@type' in d:
982-
d['type'] = d['@type']
983-
return cls(**d)
984-
985-
986-
class DatabricksError(IOError):
987-
""" Generic error from Databricks REST API """
988-
# Known ErrorDetail types
989-
_error_info_type = "type.googleapis.com/google.rpc.ErrorInfo"
990-
991-
def __init__(self,
992-
message: str = None,
993-
*,
994-
error_code: str = None,
995-
detail: str = None,
996-
status: str = None,
997-
scimType: str = None,
998-
error: str = None,
999-
retry_after_secs: int = None,
1000-
details: List[Dict[str, any]] = None,
1001-
**kwargs):
1002-
if error:
1003-
# API 1.2 has different response format, let's adapt
1004-
message = error
1005-
if detail:
1006-
# Handle SCIM error message details
1007-
# @see https://tools.ietf.org/html/rfc7644#section-3.7.3
1008-
if detail == "null":
1009-
message = "SCIM API Internal Error"
1010-
else:
1011-
message = detail
1012-
# add more context from SCIM responses
1013-
message = f"{scimType} {message}".strip(" ")
1014-
error_code = f"SCIM_{status}"
1015-
super().__init__(message if message else error)
1016-
self.error_code = error_code
1017-
self.retry_after_secs = retry_after_secs
1018-
self.details = [ErrorDetail.from_dict(detail) for detail in details] if details else []
1019-
self.kwargs = kwargs
1020-
1021-
def get_error_info(self) -> List[ErrorDetail]:
1022-
return self._get_details_by_type(DatabricksError._error_info_type)
1023-
1024-
def _get_details_by_type(self, error_type) -> List[ErrorDetail]:
1025-
if self.details == None:
1026-
return []
1027-
return [detail for detail in self.details if detail.type == error_type]
1028-
1029-
1030967
class ApiClient:
1031968
_cfg: Config
1032969
_RETRY_AFTER_DEFAULT: int = 1
@@ -1255,7 +1192,7 @@ def _make_nicer_error(self, *, response: requests.Response, **kwargs) -> Databri
12551192
if is_too_many_requests_or_unavailable:
12561193
kwargs['retry_after_secs'] = self._parse_retry_after(response)
12571194
kwargs['message'] = message
1258-
return DatabricksError(**kwargs)
1195+
return error_mapper(status_code, kwargs)
12591196

12601197
def _record_request_log(self, response: requests.Response, raw=False):
12611198
if not logger.isEnabledFor(logging.DEBUG):

‎databricks/sdk/errors.py‎

Lines changed: 0 additions & 6 deletions
This file was deleted.

‎databricks/sdk/errors/__init__.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
from .base import DatabricksError, ErrorDetail
2+
from .mapper import error_mapper
3+
from .mapping import *
4+
from .sdk import *

‎databricks/sdk/errors/base.py‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
from typing import Dict, List
2+
3+
4+
class ErrorDetail:
5+
6+
def __init__(self,
7+
type: str = None,
8+
reason: str = None,
9+
domain: str = None,
10+
metadata: dict = None,
11+
**kwargs):
12+
self.type = type
13+
self.reason = reason
14+
self.domain = domain
15+
self.metadata = metadata
16+
17+
@classmethod
18+
def from_dict(cls, d: Dict[str, any]) -> 'ErrorDetail':
19+
if '@type' in d:
20+
d['type'] = d['@type']
21+
return cls(**d)
22+
23+
24+
class DatabricksError(IOError):
25+
""" Generic error from Databricks REST API """
26+
# Known ErrorDetail types
27+
_error_info_type = "type.googleapis.com/google.rpc.ErrorInfo"
28+
29+
def __init__(self,
30+
message: str = None,
31+
*,
32+
error_code: str = None,
33+
detail: str = None,
34+
status: str = None,
35+
scimType: str = None,
36+
error: str = None,
37+
retry_after_secs: int = None,
38+
details: List[Dict[str, any]] = None,
39+
**kwargs):
40+
if error:
41+
# API 1.2 has different response format, let's adapt
42+
message = error
43+
if detail:
44+
# Handle SCIM error message details
45+
# @see https://tools.ietf.org/html/rfc7644#section-3.7.3
46+
if detail == "null":
47+
message = "SCIM API Internal Error"
48+
else:
49+
message = detail
50+
# add more context from SCIM responses
51+
message = f"{scimType} {message}".strip(" ")
52+
error_code = f"SCIM_{status}"
53+
super().__init__(message if message else error)
54+
self.error_code = error_code
55+
self.retry_after_secs = retry_after_secs
56+
self.details = [ErrorDetail.from_dict(detail) for detail in details] if details else []
57+
self.kwargs = kwargs
58+
59+
def get_error_info(self) -> List[ErrorDetail]:
60+
return self._get_details_by_type(DatabricksError._error_info_type)
61+
62+
def _get_details_by_type(self, error_type) -> List[ErrorDetail]:
63+
if self.details == None:
64+
return []
65+
return [detail for detail in self.details if detail.type == error_type]

‎databricks/sdk/errors/mapper.py‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
from databricks.sdk.errors import mapping
2+
from databricks.sdk.errors.base import DatabricksError
3+
4+
5+
def error_mapper(status_code: int, raw: dict) -> DatabricksError:
6+
error_code = raw.get('error_code', None)
7+
if error_code in mapping.ERROR_CODE_MAPPING:
8+
# more specific error codes override more generic HTTP status codes
9+
return mapping.ERROR_CODE_MAPPING[error_code](**raw)
10+
11+
if status_code in mapping.STATUS_CODE_MAPPING:
12+
# more generic HTTP status codes matched after more specific error codes,
13+
# where there's a default exception class per HTTP status code, and we do
14+
# rely on Databricks platform exception mapper to do the right thing.
15+
return mapping.STATUS_CODE_MAPPING[status_code](**raw)
16+
17+
# backwards-compatible error creation for cases like using older versions of
18+
# the SDK on way never releases of the platform.
19+
return DatabricksError(**raw)

‎databricks/sdk/errors/mapping.py‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
# Code generated from OpenAPI specs by Databricks SDK Generator. DO NOT EDIT.
2+
3+
from .base import DatabricksError
4+
5+
6+
class BadRequest(DatabricksError):
7+
"""the request is invalid"""
8+
9+
10+
class Unauthenticated(DatabricksError):
11+
"""the request does not have valid authentication (AuthN) credentials for the operation"""
12+
13+
14+
class PermissionDenied(DatabricksError):
15+
"""the caller does not have permission to execute the specified operation"""
16+
17+
18+
class NotFound(DatabricksError):
19+
"""the operation was performed on a resource that does not exist"""
20+
21+
22+
class ResourceConflict(DatabricksError):
23+
"""maps to all HTTP 409 (Conflict) responses"""
24+
25+
26+
class TooManyRequests(DatabricksError):
27+
"""maps to HTTP code: 429 Too Many Requests"""
28+
29+
30+
class Cancelled(DatabricksError):
31+
"""the operation was explicitly canceled by the caller"""
32+
33+
34+
class InternalError(DatabricksError):
35+
"""some invariants expected by the underlying system have been broken"""
36+
37+
38+
class NotImplemented(DatabricksError):
39+
"""the operation is not implemented or is not supported/enabled in this service"""
40+
41+
42+
class TemporarilyUnavailable(DatabricksError):
43+
"""the service is currently unavailable"""
44+
45+
46+
class DeadlineExceeded(DatabricksError):
47+
"""the deadline expired before the operation could complete"""
48+
49+
50+
class InvalidParameterValue(BadRequest):
51+
"""supplied value for a parameter was invalid"""
52+
53+
54+
class Aborted(ResourceConflict):
55+
"""the operation was aborted, typically due to a concurrency issue such as a sequencer check
56+
failure"""
57+
58+
59+
class AlreadyExists(ResourceConflict):
60+
"""operation was rejected due a conflict with an existing resource"""
61+
62+
63+
class ResourceAlreadyExists(ResourceConflict):
64+
"""operation was rejected due a conflict with an existing resource"""
65+
66+
67+
class ResourceExhausted(TooManyRequests):
68+
"""operation is rejected due to per-user rate limiting"""
69+
70+
71+
class RequestLimitExceeded(TooManyRequests):
72+
"""cluster request was rejected because it would exceed a resource limit"""
73+
74+
75+
class Unknown(InternalError):
76+
"""this error is used as a fallback if the platform-side mapping is missing some reason"""
77+
78+
79+
class DataLoss(InternalError):
80+
"""unrecoverable data loss or corruption"""
81+
82+
83+
STATUS_CODE_MAPPING = {
84+
400: BadRequest,
85+
401: Unauthenticated,
86+
403: PermissionDenied,
87+
404: NotFound,
88+
409: ResourceConflict,
89+
429: TooManyRequests,
90+
499: Cancelled,
91+
500: InternalError,
92+
501: NotImplemented,
93+
503: TemporarilyUnavailable,
94+
504: DeadlineExceeded,
95+
}
96+
97+
ERROR_CODE_MAPPING = {
98+
'INVALID_PARAMETER_VALUE': InvalidParameterValue,
99+
'ABORTED': Aborted,
100+
'ALREADY_EXISTS': AlreadyExists,
101+
'RESOURCE_ALREADY_EXISTS': ResourceAlreadyExists,
102+
'RESOURCE_EXHAUSTED': ResourceExhausted,
103+
'REQUEST_LIMIT_EXCEEDED': RequestLimitExceeded,
104+
'UNKNOWN': Unknown,
105+
'DATA_LOSS': DataLoss,
106+
}

‎databricks/sdk/errors/sdk.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
class OperationFailed(RuntimeError):
2+
pass
3+
4+
5+
class OperationTimeout(RuntimeError, TimeoutError):
6+
pass

‎tests/test_errors.py‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import pytest
2+
3+
from databricks.sdk import errors
4+
5+
6+
def test_error_code_has_precedence_over_http_status():
7+
err = errors.error_mapper(400, {'error_code': 'INVALID_PARAMETER_VALUE', 'message': 'nope'})
8+
assert errors.InvalidParameterValue == type(err)
9+
10+
11+
def test_http_status_code_maps_fine():
12+
err = errors.error_mapper(400, {'error_code': 'MALFORMED_REQUEST', 'message': 'nope'})
13+
assert errors.BadRequest == type(err)
14+
15+
16+
def test_other_errors_also_map_fine():
17+
err = errors.error_mapper(417, {'error_code': 'WHOOPS', 'message': 'nope'})
18+
assert errors.DatabricksError == type(err)
19+
20+
21+
def test_missing_error_code():
22+
err = errors.error_mapper(522, {'message': 'nope'})
23+
assert errors.DatabricksError == type(err)
24+
25+
26+
@pytest.mark.parametrize('status_code, error_code, klass',
27+
[(400, ..., errors.BadRequest), (400, 'INVALID_PARAMETER_VALUE', errors.BadRequest),
28+
(400, 'INVALID_PARAMETER_VALUE', errors.InvalidParameterValue),
29+
(400, 'REQUEST_LIMIT_EXCEEDED', errors.TooManyRequests), (400, ..., IOError),
30+
(401, ..., errors.Unauthenticated), (401, ..., IOError),
31+
(403, ..., errors.PermissionDenied),
32+
(403, ..., IOError), (404, ..., errors.NotFound), (404, ..., IOError),
33+
(409, ..., errors.ResourceConflict), (409, 'ABORTED', errors.Aborted),
34+
(409, 'ABORTED', errors.ResourceConflict),
35+
(409, 'ALREADY_EXISTS', errors.AlreadyExists),
36+
(409, 'ALREADY_EXISTS', errors.ResourceConflict), (409, ..., IOError),
37+
(429, ..., errors.TooManyRequests),
38+
(429, 'REQUEST_LIMIT_EXCEEDED', errors.TooManyRequests),
39+
(429, 'REQUEST_LIMIT_EXCEEDED', errors.RequestLimitExceeded),
40+
(429, 'RESOURCE_EXHAUSTED', errors.TooManyRequests),
41+
(429, 'RESOURCE_EXHAUSTED', errors.ResourceExhausted), (429, ..., IOError),
42+
(499, ..., errors.Cancelled), (499, ..., IOError), (500, ..., errors.InternalError),
43+
(500, 'UNKNOWN', errors.InternalError), (500, 'UNKNOWN', errors.Unknown),
44+
(500, 'DATA_LOSS', errors.InternalError), (500, 'DATA_LOSS', errors.DataLoss),
45+
(500, ..., IOError), (501, ..., errors.NotImplemented), (501, ..., IOError),
46+
(503, ..., errors.TemporarilyUnavailable), (503, ..., IOError),
47+
(504, ..., errors.DeadlineExceeded), (504, ..., IOError),
48+
(444, ..., errors.DatabricksError), (444, ..., IOError), ])
49+
def test_subclasses(status_code, error_code, klass):
50+
try:
51+
raise errors.error_mapper(status_code, {'error_code': error_code, 'message': 'nope'})
52+
except klass:
53+
return

0 commit comments

Comments
 (0)