Fixed error handling for SCIM and CommandExecution APIs (#94)

## Changes

- `DatabricksError` and `_make_nicer_error` were not integration tested
before
- this PR adds proper kwarg resolution

## Tests

- [x] `make test` run locally
- [x] `make fmt` applied
- [x] relevant integration tests applied

Author: nfx

databricks/sdk/core.py

@@ -689,10 +689,10 @@ def __init__(self,
                  scimType: str = None,
                  error: str = None,
                  **kwargs):
-        if not message and error:
+        if error:
             # API 1.2 has different response format, let's adapt
             message = error
-        if not message and detail:
+        if detail:
             # Handle SCIM error message details
             # @see https://tools.ietf.org/html/rfc7644#section-3.7.3
             if detail == "null":
@@ -758,7 +758,7 @@ def do(self, method: str, path: str, query: dict = None, body: dict = None) -> d
             message = self._make_sense_from_html(response.text)
             if not message:
                 message = response.reason
-            raise self._make_nicer_error(message) from None
+            raise self._make_nicer_error(message=message) from None
 
     @staticmethod
     def _make_sense_from_html(txt: str) -> str:
@@ -771,11 +771,13 @@ def _make_sense_from_html(txt: str) -> str:
             return match.group(1).strip()
         return txt
 
-    def _make_nicer_error(self, message: str, status_code: int = 200, **kwargs) -> DatabricksError:
+    def _make_nicer_error(self, status_code: int = 200, **kwargs) -> DatabricksError:
+        message = kwargs.get('message', 'request failed')
         is_http_unauthorized_or_forbidden = status_code in (401, 403)
         if is_http_unauthorized_or_forbidden:
             message = self._cfg.wrap_debug_info(message)
-        return DatabricksError(message, **kwargs)
+        kwargs['message'] = message
+        return DatabricksError(**kwargs)
 
     def _record_request_log(self, response: requests.Response):
         if not logger.isEnabledFor(logging.DEBUG):

tests/integration/test_clusters.py

@@ -1,6 +1,10 @@
 import logging
 from datetime import timedelta
 
+import pytest
+
+from databricks.sdk.core import DatabricksError
+
 
 def test_smallest_node_type(w):
     node_type_id = w.clusters.select_node_type(local_disk=True)
@@ -33,3 +37,11 @@ def test_create_cluster(w, env_or_skip, random):
                              num_workers=1,
                              timeout=timedelta(minutes=10))
     logging.info(f'Created: {info}')
+
+
+def test_error_unmarshall(w, random):
+    with pytest.raises(DatabricksError) as exc_info:
+        w.clusters.get('__non_existing__')
+    err = exc_info.value
+    assert 'Cluster __non_existing__ does not exist' in str(err)
+    assert 'INVALID_PARAMETER_VALUE' == err.error_code

tests/integration/test_commands.py

@@ -0,0 +1,11 @@
+import pytest
+
+from databricks.sdk.core import DatabricksError
+
+
+def test_error_unmarshall(w, random):
+    with pytest.raises(DatabricksError) as exc_info:
+        w.command_execution.execute(cluster_id='__non_existing__')
+    err = exc_info.value
+    assert 'requirement failed: missing contextId' in str(err)
+    assert err.error_code is None

tests/integration/test_groups.py

@@ -0,0 +1,15 @@
+import pytest
+
+from databricks.sdk.core import DatabricksError
+
+
+def test_filtering_groups(w, random):
+    all = w.groups.list(filter=f'displayName eq any-{random(12)}')
+    found = len(list(all))
+    assert found == 0
+
+
+def test_scim_error_unmarshall(w, random):
+    with pytest.raises(DatabricksError) as exc_info:
+        w.groups.list(filter=random(12))
+    assert 'Given filter operator is not supported' in str(exc_info.value)