Skip to content

Commit 4691a89

Browse files
authored
Better error handling in Repos (#428)
In current version, we have "catch-all" error handling that efficiently hides the actual reason for the error. It's not possible to understand the reason for error, for example, when IP Access List is used. This patch improves the error reporting: Without patch ``` Error: RuntimeError: Can't find repo ID for /Repos/user/repo ``` With patch: ``` Error: RuntimeError: Error fetching repo ID for /Repos/user/repo: HTTP code: 403, Unauthorized access to Org: 1234567890 ```
1 parent 4a6d25a commit 4691a89

File tree

3 files changed

+29
-6
lines changed

3 files changed

+29
-6
lines changed

databricks_cli/repos/api.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,22 @@ def get_repo_id(self, path):
4040
status = self.ws_client.get_status(path)
4141
if status['object_type'] == 'REPO':
4242
return status['object_id']
43-
except requests.exceptions.HTTPError:
44-
pass
43+
except requests.exceptions.HTTPError as ex:
44+
if ex.response.status_code != 404:
45+
jsn = ex.response.json()
46+
if 'message' in jsn:
47+
msg = jsn['message']
48+
else:
49+
msg = ex.response.reason
50+
raise RuntimeError(
51+
"Error fetching repo ID for {path}: HTTP code: {code}, {ex}".format(
52+
path=path, code=ex.response.status_code, ex=msg))
4553

4654
raise RuntimeError("Can't find repo ID for {path}".format(path=path))
4755

4856
def list(self, path_prefix, next_page_token):
4957
"""
50-
List repos that the caller has Manage permissions on. Results are
58+
List repos that the caller has Manage permissions on. Results are
5159
paginated with each page containing twenty repos.
5260
"""
5361
return self.client.list_repos(path_prefix, next_page_token)
@@ -66,7 +74,7 @@ def get(self, repo_id):
6674

6775
def update(self, repo_id, branch, tag):
6876
"""
69-
Checks out the repo to the given branch or tag. Only one of ``branch``
77+
Checks out the repo to the given branch or tag. Only one of ``branch``
7078
or ``tag`` should be provided.
7179
"""
7280
assert bool(branch is not None) ^ bool(tag is not None)

tests/repos/test_api.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,12 @@
2222
# limitations under the License.
2323

2424
# pylint:disable=redefined-outer-name
25+
import json
2526

2627
import mock
2728

2829
import pytest
30+
import requests
2931

3032
import databricks_cli.repos.api as api
3133
import databricks_cli.workspace.api as workspace_api
@@ -44,6 +46,10 @@
4446
'created_at': 124,
4547
'object_id': TEST_ID,
4648
}
49+
TEST_ERROR_JSON_RESPONSE = {
50+
'error': "Permission Denied",
51+
'message': "Unauthorized access to Org: 123456",
52+
}
4753

4854

4955
@pytest.fixture()
@@ -72,4 +78,13 @@ def test_get_id_path_not_in_repos(self, repos_api_with_ws_service):
7278
def test_get_id_not_a_repo(self, repos_api_with_ws_service):
7379
repos_api_with_ws_service.ws_client.get_status.return_value = TEST_NON_REPO_JSON_RESPONSE
7480
with pytest.raises(RuntimeError):
75-
repos_api_with_ws_service.get_repo_id(TEST_PATH)
81+
repos_api_with_ws_service.get_repo_id(TEST_PATH)
82+
83+
def test_get_id_other_error(self, repos_api_with_ws_service):
84+
response = requests.Response()
85+
response.status_code = 403
86+
response._content = json.dumps(TEST_ERROR_JSON_RESPONSE).encode() # NOQA
87+
repos_api_with_ws_service.ws_client.get_status.side_effect = mock.Mock(
88+
side_effect=requests.exceptions.HTTPError(response=response))
89+
with pytest.raises(RuntimeError):
90+
repos_api_with_ws_service.get_repo_id(TEST_PATH)

tests/repos/test_cli.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,4 +139,4 @@ def test_delete_validation(repos_api_mock):
139139
["--repo-id", TEST_ID])
140140
repos_api_mock.delete.assert_called_once_with(
141141
TEST_ID,
142-
)
142+
)

0 commit comments

Comments
 (0)