Skip to content

Commit

Permalink
Fix/response (#57)
Browse files Browse the repository at this point in the history
* fix(response): make delete_group return empty dict if google returned empty string

* fix(response): fix return type in docstring for get_all_groups

* black

* fix(response): document idiosyncrasy
  • Loading branch information
vpsx authored Mar 1, 2019
1 parent 7542bd1 commit 5fe661b
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 8 deletions.
13 changes: 7 additions & 6 deletions cirrus/google_cloud/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ def get_reason(http_error):
reason = ""
return reason


def exception_do_not_retry(e):
"""
True if we should not retry.
Expand Down Expand Up @@ -165,8 +165,7 @@ def exception_do_not_retry(e):
# Valid rate limit reasons from IAM API
# IAM API doesn't seem to return rate-limit 403s.
return (
reason not in resource_rlreasons
and reason not in directory_rlreasons
reason not in resource_rlreasons and reason not in directory_rlreasons
)
return False

Expand Down Expand Up @@ -1101,8 +1100,8 @@ def get_all_groups(self):
Return a list of all groups in the domain
Returns:
dict: JSON response from API call, which should contain a list
of groups
list(dict): groups field of JSON response from API call,
which should contain a list of groups
`Google API Reference <https://developers.google.com/admin-sdk/directory/v1/reference/groups/list>`_
.. code-block:: python
Expand Down Expand Up @@ -1389,7 +1388,9 @@ def delete_group(self, group_id):
`Google API Reference <https://developers.google.com/admin-sdk/directory/v1/reference/groups/delete>`_
"""
try:
return self._admin_service.groups().delete(groupKey=group_id).execute()
r = self._admin_service.groups().delete(groupKey=group_id).execute()
# Directory API's notion of empty response body is "", differing from other APIs' {}
return {} if r == "" else r
except GoogleHttpError as err:
if err.resp.status == 404:
# not found, group doesn't exist. This is fine
Expand Down
7 changes: 5 additions & 2 deletions test/test_google_cloud_manage.py
Original file line number Diff line number Diff line change
Expand Up @@ -1030,14 +1030,17 @@ def test_get_all_groups_pagination(test_cloud_manager):
assert kwargs["pageToken"] == next_page_token


def test_delete_group(test_cloud_manager):
@pytest.mark.parametrize("google_return_value", [{}, ""])
def test_delete_group(test_cloud_manager, google_return_value):
"""
Test that deleting a group return the ID from the API response and that
the API is called with the correct values
"""
# Setup #
group_id = "123"
mock_config = {"groups.return_value.delete.return_value.execute.return_value": {}}
mock_config = {
"groups.return_value.delete.return_value.execute.return_value": google_return_value
}
test_cloud_manager._admin_service.configure_mock(**mock_config)

# Call #
Expand Down

0 comments on commit 5fe661b

Please sign in to comment.