Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tdl 16369 revert back api access change #90

Merged
merged 7 commits into from
Nov 19, 2021
Merged

Tdl 16369 revert back api access change #90

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5ecb131
revert back check api changes
prijendev Nov 16, 2021
37b922f
resolvedd pylint error
prijendev Nov 16, 2021
12b448f
updated unittest cases
prijendev Nov 16, 2021
cb1051d
Updated error message
prijendev Nov 16, 2021
7cf598b
updated error message
prijendev Nov 16, 2021
010f7c9
Added timeout for circleci
Nov 16, 2021
2f1803d
Increased the timeout to 200minutes
Nov 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions tap_zendesk/discover.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,20 @@ def discover_streams(client, config):
streams.append({'stream': stream.name, 'tap_stream_id': stream.name, 'schema': schema, 'metadata': stream.load_metadata()})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@prijendev If the stream would not have 'read' access, would we enter the JSON for that stream in the catalog?

Copy link
Contributor Author

@prijendev prijendev Nov 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we would enter the JSON for that stream in catalog


if error_list:

total_stream = len(STREAMS.values()) # Total no of streams
streams_name = ", ".join(error_list)
message = "HTTP-error-code: 403, Error: You are missing the following required scopes: read. "\
if len(error_list) != total_stream:
message = "The account credentials supplied do not have 'read' access to the following stream(s): {}. "\
"The data for these streams would not be collected due to lack of required permission.".format(streams_name)
# If atleast one stream have read permission then just print warning message for all streams
# which does not have read permission
LOGGER.warning(message)
else:
message = "HTTP-error-code: 403, Error: You are missing the following required scopes: read. "\
"The account credentials supplied do not have read access for the following stream(s): {}".format(streams_name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@prijendev Can you also update this message as,
"HTTP-error-code: 403, Error: The account credentials supplied do not have 'read' access to any of streams supported by the tap. Data collection cannot be initiated due to lack of permissions."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

raise ZendeskForbiddenError(message)
# If none of the streams are having the 'read' access, then the code will raise an error
raise ZendeskForbiddenError(message)


return streams
123 changes: 78 additions & 45 deletions test/unittests/test_discovery_mode.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class TestDiscovery(unittest.TestCase):
'''
Test that we can call api for each stream in discovey mode and handle forbidden error.
'''
@patch("tap_zendesk.discover.LOGGER.warning")
@patch('tap_zendesk.streams.Organizations.check_access',side_effect=zenpy.lib.exception.APIException(ACCSESS_TOKEN_ERROR))
@patch('tap_zendesk.streams.Users.check_access',side_effect=zenpy.lib.exception.APIException(ACCSESS_TOKEN_ERROR))
@patch('tap_zendesk.streams.TicketForms.check_access',side_effect=zenpy.lib.exception.APIException(ACCSESS_TOKEN_ERROR))
Expand All @@ -47,32 +48,28 @@ class TestDiscovery(unittest.TestCase):
mocked_get(status_code=403, json={"key1": "val1"}), # Response of the 9th get request call
mocked_get(status_code=403, json={"key1": "val1"}) # Response of the 10th get request call
])
def test_discovery_handles_403__raise_tap_zendesk_forbidden_error(self, mock_get, mock_resolve_schema_references,
mock_load_metadata, mock_load_schema,mock_load_shared_schema_refs, mocked_sla_policies,
mocked_ticket_forms, mock_users, mock_organizations):
def test_discovery_handles_403__raise_tap_zendesk_forbidden_error(self, mock_get, mock_resolve_schema_references,
mock_load_metadata, mock_load_schema,mock_load_shared_schema_refs, mocked_sla_policies,
mocked_ticket_forms, mock_users, mock_organizations, mock_logger):
'''
Test that we handle forbidden error for child streams. discover_streams calls check_access for each stream to
check the read perission. discover_streams call many other methods including load_shared_schema_refs, load_metadata,
load_schema, resolve_schema_references also which we mock to test forbidden error. We mock check_access method of
Test that we handle forbidden error for child streams. discover_streams calls check_access for each stream to
check the read perission. discover_streams call many other methods including load_shared_schema_refs, load_metadata,
load_schema, resolve_schema_references also which we mock to test forbidden error. We mock check_access method of
some of stream method which call request of zenpy module and also mock get method of requests module with 200, 403 error.

'''
try:
responses = discover.discover_streams('dummy_client', {'subdomain': 'arp', 'access_token': 'dummy_token', 'start_date':START_DATE})
except tap_zendesk.http.ZendeskForbiddenError as e:
expected_error_message = "HTTP-error-code: 403, Error: You are missing the following required scopes: read. "\
"The account credentials supplied do not have read access for the following stream(s): groups, users, "\
"organizations, ticket_audits, ticket_comments, ticket_fields, ticket_forms, group_memberships, macros, "\
"satisfaction_ratings, tags, ticket_metrics"

# Verifying the message formed for the custom exception
self.assertEqual(str(e), expected_error_message)

discover.discover_streams('dummy_client', {'subdomain': 'arp', 'access_token': 'dummy_token', 'start_date':START_DATE})
expected_call_count = 10
actual_call_count = mock_get.call_count
self.assertEqual(expected_call_count, actual_call_count)


# Verifying the logger message
mock_logger.assert_called_with("The account credentials supplied do not have 'read' access to the following stream(s): "\
"groups, users, organizations, ticket_audits, ticket_comments, ticket_fields, ticket_forms, group_memberships, macros, "\
"satisfaction_ratings, tags, ticket_metrics. The data for these streams would not be collected due to lack of required "\
"permission.")

@patch("tap_zendesk.discover.LOGGER.warning")
@patch('tap_zendesk.streams.Organizations.check_access',side_effect=zenpy.lib.exception.APIException(ACCSESS_TOKEN_ERROR))
@patch('tap_zendesk.streams.Users.check_access',side_effect=zenpy.lib.exception.APIException(ACCSESS_TOKEN_ERROR))
@patch('tap_zendesk.streams.TicketForms.check_access',side_effect=zenpy.lib.exception.APIException(ACCSESS_TOKEN_ERROR))
Expand All @@ -94,31 +91,29 @@ def test_discovery_handles_403__raise_tap_zendesk_forbidden_error(self, mock_get
mocked_get(status_code=403, json={"key1": "val1"}), # Response of the 9th get request call
mocked_get(status_code=403, json={"key1": "val1"}) # Response of the 10th get request call
])
def test_discovery_handles_403_raise_zenpy_forbidden_error_for_access_token(self, mock_get, mock_resolve_schema_references, mock_load_metadata,
mock_load_schema,mock_load_shared_schema_refs, mocked_sla_policies, mocked_ticket_forms,
mock_users, mock_organizations):
def test_discovery_handles_403_raise_zenpy_forbidden_error_for_access_token(self, mock_get, mock_resolve_schema_references, mock_load_metadata,
mock_load_schema,mock_load_shared_schema_refs, mocked_sla_policies, mocked_ticket_forms,
mock_users, mock_organizations, mock_logger):
'''
Test that we handle forbidden error received from last failed request which we called from zenpy module and
raised zenpy.lib.exception.APIException. discover_streams calls check_access for each stream to check the
read perission. discover_streams call many other methods including load_shared_schema_refs, load_metadata,
load_schema, resolve_schema_references also which we mock to test forbidden error. We mock check_access method of
log proper warning message. discover_streams calls check_access for each stream to check the
read perission. discover_streams call many other methods including load_shared_schema_refs, load_metadata,
load_schema, resolve_schema_references also which we mock to test forbidden error. We mock check_access method of
some of stream method which call request of zenpy module and also mock get method of requests module with 200, 403 error.
'''
try:
responses = discover.discover_streams('dummy_client', {'subdomain': 'arp', 'access_token': 'dummy_token', 'start_date':START_DATE})
except tap_zendesk.http.ZendeskForbiddenError as e:
expected_error_message = "HTTP-error-code: 403, Error: You are missing the following required scopes: read. "\
"The account credentials supplied do not have read access for the following stream(s): groups, users, "\
"organizations, ticket_audits, ticket_comments, ticket_fields, ticket_forms, group_memberships, macros, "\
"satisfaction_ratings, tags, ticket_metrics, sla_policies"

self.assertEqual(str(e), expected_error_message)
discover.discover_streams('dummy_client', {'subdomain': 'arp', 'access_token': 'dummy_token', 'start_date':START_DATE})

expected_call_count = 10
actual_call_count = mock_get.call_count
self.assertEqual(expected_call_count, actual_call_count)

# Verifying the logger message
mock_logger.assert_called_with("The account credentials supplied do not have 'read' access to the following stream(s): "\
"groups, users, organizations, ticket_audits, ticket_comments, ticket_fields, ticket_forms, group_memberships, macros, "\
"satisfaction_ratings, tags, ticket_metrics, sla_policies. The data for these streams would not be collected due to "\
"lack of required permission.")


@patch("tap_zendesk.discover.LOGGER.warning")
@patch('tap_zendesk.streams.Organizations.check_access',side_effect=zenpy.lib.exception.APIException(API_TOKEN_ERROR))
@patch('tap_zendesk.streams.Users.check_access',side_effect=zenpy.lib.exception.APIException(API_TOKEN_ERROR))
@patch('tap_zendesk.streams.TicketForms.check_access',side_effect=zenpy.lib.exception.APIException(API_TOKEN_ERROR))
Expand All @@ -142,27 +137,24 @@ def test_discovery_handles_403_raise_zenpy_forbidden_error_for_access_token(self
])
def test_discovery_handles_403_raise_zenpy_forbidden_error_for_api_token(self, mock_get, mock_resolve_schema_references,
mock_load_metadata, mock_load_schema,mock_load_shared_schema_refs, mocked_sla_policies,
mocked_ticket_forms, mock_users, mock_organizations):
mocked_ticket_forms, mock_users, mock_organizations, mock_logger):
'''
Test that we handle forbidden error received from last failed request which we called from zenpy module and
raised zenpy.lib.exception.APIException. discover_streams calls check_access for each stream to check the
log proper warning message. discover_streams calls check_access for each stream to check the
read perission. discover_streams call many other methods including load_shared_schema_refs, load_metadata,
load_schema, resolve_schema_references also which we mock to test forbidden error. We mock check_access method of
some of stream method which call request of zenpy module and also mock get method of requests module with 200, 403 error.
'''
try:
responses = discover.discover_streams('dummy_client', {'subdomain': 'arp', 'access_token': 'dummy_token', 'start_date':START_DATE})
except tap_zendesk.http.ZendeskForbiddenError as e:
expected_error_message = "HTTP-error-code: 403, Error: You are missing the following required scopes: read. "\
"The account credentials supplied do not have read access for the following stream(s): tickets, groups, users, "\
"organizations, ticket_fields, ticket_forms, group_memberships, macros, satisfaction_ratings, tags"

self.assertEqual(str(e), expected_error_message)

responses = discover.discover_streams('dummy_client', {'subdomain': 'arp', 'access_token': 'dummy_token', 'start_date':START_DATE})
expected_call_count = 10
actual_call_count = mock_get.call_count
self.assertEqual(expected_call_count, actual_call_count)


# Verifying the logger message
mock_logger.assert_called_with("The account credentials supplied do not have 'read' access to the following stream(s): "\
"tickets, groups, users, organizations, ticket_fields, ticket_forms, group_memberships, macros, satisfaction_ratings, "\
"tags. The data for these streams would not be collected due to lack of required permission.")

@patch('tap_zendesk.streams.Organizations.check_access',side_effect=zenpy.lib.exception.APIException(ACCSESS_TOKEN_ERROR))
@patch('tap_zendesk.streams.Users.check_access',side_effect=zenpy.lib.exception.APIException(ACCSESS_TOKEN_ERROR))
Expand Down Expand Up @@ -274,3 +266,44 @@ def test_discovery_handles_200_response(self, mock_get, mock_resolve_schema_refe
expected_call_count = 10
actual_call_count = mock_get.call_count
self.assertEqual(expected_call_count, actual_call_count)

@patch("tap_zendesk.discover.LOGGER.warning")
@patch('tap_zendesk.streams.Organizations.check_access',side_effect=zenpy.lib.exception.APIException(API_TOKEN_ERROR))
@patch('tap_zendesk.streams.Users.check_access',side_effect=zenpy.lib.exception.APIException(API_TOKEN_ERROR))
@patch('tap_zendesk.streams.TicketForms.check_access',side_effect=zenpy.lib.exception.APIException(API_TOKEN_ERROR))
@patch('tap_zendesk.streams.SLAPolicies.check_access',side_effect=zenpy.lib.exception.APIException(API_TOKEN_ERROR))
@patch('tap_zendesk.discover.load_shared_schema_refs', return_value={})
@patch('tap_zendesk.streams.Stream.load_metadata', return_value={})
@patch('tap_zendesk.streams.Stream.load_schema', return_value={})
@patch('singer.resolve_schema_references', return_value={})
@patch('requests.get',
side_effect=[
mocked_get(status_code=403, json={"key1": "val1"}), # Response of the 1st get request call
mocked_get(status_code=403, json={"key1": "val1"}), # Response of the 2nd get request call
mocked_get(status_code=403, json={"key1": "val1"}), # Response of the 3rd get request call
mocked_get(status_code=403, json={"key1": "val1"}), # Response of the 4th get request call
mocked_get(status_code=403, json={"key1": "val1"}), # Response of the 5th get request call
mocked_get(status_code=403, json={"key1": "val1"}), # Response of the 6th get request call
mocked_get(status_code=403, json={"key1": "val1"}), # Response of the 7th get request call
mocked_get(status_code=403, json={"key1": "val1"}), # Response of the 8th get request call
mocked_get(status_code=403, json={"key1": "val1"}), # Response of the 9th get request call
mocked_get(status_code=403, json={"key1": "val1"}) # Response of the 10th get request call
])
def test_discovery_handles_403_for_all_streams_api_token(self, mock_get, mock_resolve_schema_references,
mock_load_metadata, mock_load_schema,mock_load_shared_schema_refs, mocked_sla_policies,
mocked_ticket_forms, mock_users, mock_organizations, mock_logger):
'''
Test that we handle forbidden error received from all streams and raise the ZendeskForbiddenError
with proper error message. discover_streams calls check_access for each stream to check the
read perission. discover_streams call many other methods including load_shared_schema_refs, load_metadata,
load_schema, resolve_schema_references also which we mock to test forbidden error. We mock check_access method of
some of stream method which call request of zenpy module and also mock get method of requests module with 200, 403 error.
'''
try:
responses = discover.discover_streams('dummy_client', {'subdomain': 'arp', 'access_token': 'dummy_token', 'start_date':START_DATE})
except http.ZendeskForbiddenError as e:
expected_message = "HTTP-error-code: 403, Error: You are missing the following required scopes: read. The account credentials "\
"supplied do not have read access for the following stream(s): tickets, groups, users, organizations, ticket_audits, "\
"ticket_comments, ticket_fields, ticket_forms, group_memberships, macros, satisfaction_ratings, tags, ticket_metrics, sla_policies"
# # Verifying the message formed for the custom exception
self.assertEqual(str(e), expected_message)