diff --git a/tap_github/__init__.py b/tap_github/__init__.py index 36855fec..6047211e 100644 --- a/tap_github/__init__.py +++ b/tap_github/__init__.py @@ -97,7 +97,7 @@ class RateLimitExceeded(GithubException): }, 404: { "raise_exception": NotFoundException, - "message": "The resource you have specified cannot be found." + "message": "The resource you have specified cannot be found" }, 409: { "raise_exception": ConflictError, @@ -164,25 +164,32 @@ def get_bookmark(state, repo, stream_name, bookmark_key, start_date): return start_date return None -def raise_for_error(resp): +def raise_for_error(resp, source): + + content_length = len(resp.content) + if content_length == 0: + # There is nothing we can do here since Github has neither sent + # us a 2xx response nor a response content. + return + + error_code = resp.status_code try: - resp.raise_for_status() - except (requests.HTTPError, requests.ConnectionError) as error: - try: - error_code = resp.status_code - try: - response_json = resp.json() - except Exception: - response_json = {} - - message = "HTTP-error-code: {}, Error: {}".format( - error_code, ERROR_CODE_EXCEPTION_MAPPING.get(error_code, {}).get("message", "Unknown Error") if response_json == {} else response_json) - - exc = ERROR_CODE_EXCEPTION_MAPPING.get(error_code, {}).get("raise_exception", GithubException) - raise exc(message) from None - - except (ValueError, TypeError): - raise GithubException(error) from None + response_json = resp.json() + except Exception: + response_json = {} + + if error_code == 404: + details = ERROR_CODE_EXCEPTION_MAPPING.get(error_code).get("message") + if source == "teams": + details += ' or it is a personal account repository' + message = "HTTP-error-code: 404, Error: {}. Please refer \'{}\' for more details.".format(details, response_json.get("documentation_url")) + else: + message = "HTTP-error-code: {}, Error: {}".format( + error_code, ERROR_CODE_EXCEPTION_MAPPING.get(error_code, {}).get("message", "Unknown Error") if response_json == {} else response_json) + + exc = ERROR_CODE_EXCEPTION_MAPPING.get(error_code, {}).get("raise_exception", GithubException) + raise exc(message) from None + def calculate_seconds(epoch): current = time.time() return int(round((epoch - current), 0)) @@ -204,7 +211,7 @@ def authed_get(source, url, headers={}): session.headers.update(headers) resp = session.request(method='get', url=url) if resp.status_code != 200: - raise_for_error(resp) + raise_for_error(resp, source) return None else: timer.tags[metrics.Tag.http_status_code] = resp.status_code @@ -318,15 +325,7 @@ def verify_repo_access(url_for_repo, repo): message = "HTTP-error-code: 404, Error: Please check the repository name \'{}\' or you do not have sufficient permissions to access this repository.".format(repo) raise NotFoundException(message) from None -def verify_org_access(url_for_org, org): - try: - authed_get("verifying organization access", url_for_org) - except NotFoundException: - # throwing user-friendly error message as it shows "Not Found" message - message = "HTTP-error-code: 404, Error: \'{}\' is not an organization.".format(org) - raise NotFoundException(message) from None - -def verify_access_for_repo_org(config): +def verify_access_for_repo(config): access_token = config['access_token'] session.headers.update({'authorization': 'token ' + access_token, 'per_page': '1', 'page': '1'}) @@ -335,18 +334,14 @@ def verify_access_for_repo_org(config): for repo in repositories: logger.info("Verifying access of repository: %s", repo) - org = repo.split('/')[0] - url_for_repo = "https://api.github.com/repos/{}/collaborators".format(repo) - url_for_org = "https://api.github.com/orgs/{}/teams".format(org) + url_for_repo = "https://api.github.com/repos/{}/commits".format(repo) # Verifying for Repo access verify_repo_access(url_for_repo, repo) - # Verifying for Org access - verify_org_access(url_for_org, org) def do_discover(config): - verify_access_for_repo_org(config) + verify_access_for_repo(config) catalog = get_catalog() # dump catalog print(json.dumps(catalog, indent=2)) diff --git a/tests/unittests/test_exception_handling.py b/tests/unittests/test_exception_handling.py index 9247f58e..c2c5502f 100644 --- a/tests/unittests/test_exception_handling.py +++ b/tests/unittests/test_exception_handling.py @@ -9,6 +9,7 @@ def __init__(self, status_code, json, raise_error, headers={'X-RateLimit-Remaini self.raise_error = raise_error self.text = json self.headers = headers + self.content = "github" def raise_for_status(self): if not self.raise_error: @@ -49,12 +50,21 @@ def test_403_error(self, mocked_request): self.assertEquals(str(e), "HTTP-error-code: 403, Error: User doesn't have permission to access the resource.") def test_404_error(self, mocked_request): - mocked_request.return_value = get_response(404, raise_error = True) + json = {"message": "Not Found", "documentation_url": "https:/docs.github.com/"} + mocked_request.return_value = get_response(404, json = json, raise_error = True) try: tap_github.authed_get("", "") except tap_github.NotFoundException as e: - self.assertEquals(str(e), "HTTP-error-code: 404, Error: The resource you have specified cannot be found.") + self.assertEquals(str(e), "HTTP-error-code: 404, Error: The resource you have specified cannot be found. Please refer '{}' for more details.".format(json.get("documentation_url"))) + + def test_404_error_for_teams(self, mocked_request): + json = {"message": "Not Found", "documentation_url": "https:/docs.github.com/"} + + try: + tap_github.raise_for_error(get_response(404, json = json, raise_error = True), "teams") + except tap_github.NotFoundException as e: + self.assertEquals(str(e), "HTTP-error-code: 404, Error: The resource you have specified cannot be found or it is a personal account repository. Please refer '{}' for more details.".format(json.get("documentation_url"))) def test_500_error(self, mocked_request): mocked_request.return_value = get_response(500, raise_error = True) diff --git a/tests/unittests/test_formatting_dates.py b/tests/unittests/test_formatting_dates.py index 25b82183..701b2714 100644 --- a/tests/unittests/test_formatting_dates.py +++ b/tests/unittests/test_formatting_dates.py @@ -7,6 +7,7 @@ class Mockresponse: def __init__(self, resp, not_list=False): self.not_list = not_list self.json_data = resp + self.content = "github" def json(self): if self.not_list: @@ -34,7 +35,7 @@ def test_due_on_none_without_state(self, mocked_request): init_state = {} repo_path = "singer-io/tap-github" - final_state = tap_github.get_all_issue_milestones({}, repo_path, init_state, {}) + final_state = tap_github.get_all_issue_milestones({}, repo_path, init_state, {}, "") # as we will get 1 record and initial bookmark is empty, checking that if bookmark exists in state file returned self.assertTrue(final_state["bookmarks"][repo_path]["issue_milestones"]["since"]) @@ -51,7 +52,7 @@ def test_due_on_none_with_state(self, mocked_request): init_state = {'bookmarks': {'singer-io/tap-github': {'issue_milestones': {'since': '2021-05-05T07:20:36.887412Z'}}}} init_bookmark = singer.utils.strptime_to_utc(init_state["bookmarks"][repo_path]["issue_milestones"]["since"]) - final_state = tap_github.get_all_issue_milestones({}, repo_path, init_state, {}) + final_state = tap_github.get_all_issue_milestones({}, repo_path, init_state, {}, "") last_bookmark = singer.utils.strptime_to_utc(final_state["bookmarks"][repo_path]["issue_milestones"]["since"]) # as we will get 1 record, final bookmark will be greater than initial bookmark self.assertGreater(last_bookmark, init_bookmark) @@ -70,7 +71,7 @@ def test_due_on_not_none_1(self, mocked_request): init_state = {'bookmarks': {'singer-io/tap-github': {'issue_milestones': {'since': '2021-05-05T07:20:36.887412Z'}}}} init_bookmark = singer.utils.strptime_to_utc(init_state["bookmarks"][repo_path]["issue_milestones"]["since"]) - final_state = tap_github.get_all_issue_milestones({}, repo_path, init_state, {}) + final_state = tap_github.get_all_issue_milestones({}, repo_path, init_state, {}, "") last_bookmark = singer.utils.strptime_to_utc(final_state["bookmarks"][repo_path]["issue_milestones"]["since"]) # as we will get 1 record, final bookmark will be greater than initial bookmark self.assertGreater(last_bookmark, init_bookmark) @@ -88,7 +89,7 @@ def test_due_on_not_none_2(self, mocked_request): init_state = {'bookmarks': {'singer-io/tap-github': {'issue_milestones': {'since': '2021-05-08T07:20:36.887412Z'}}}} init_bookmark = init_state["bookmarks"][repo_path]["issue_milestones"]["since"] - final_state = tap_github.get_all_issue_milestones({}, repo_path, init_state, {}) + final_state = tap_github.get_all_issue_milestones({}, repo_path, init_state, {}, "") # as we will get 0 records, initial and final bookmark will be same self.assertEquals(init_bookmark, final_state["bookmarks"][repo_path]["issue_milestones"]["since"]) @@ -111,7 +112,7 @@ def test_data_containing_both_values(self, mocked_write_record, mocked_request): init_state = {'bookmarks': {'singer-io/tap-github': {'issue_milestones': {'since': '2021-05-08T07:20:36.887412Z'}}}} init_bookmark = singer.utils.strptime_to_utc(init_state["bookmarks"][repo_path]["issue_milestones"]["since"]) - final_state = tap_github.get_all_issue_milestones({}, repo_path, init_state, {}) + final_state = tap_github.get_all_issue_milestones({}, repo_path, init_state, {}, "") last_bookmark = singer.utils.strptime_to_utc(final_state["bookmarks"][repo_path]["issue_milestones"]["since"]) # as we will get 2 record, final bookmark will be greater than initial bookmark self.assertGreater(last_bookmark, init_bookmark) diff --git a/tests/unittests/test_key_error.py b/tests/unittests/test_key_error.py index 44d9cc86..ab23600d 100644 --- a/tests/unittests/test_key_error.py +++ b/tests/unittests/test_key_error.py @@ -5,6 +5,7 @@ class Mockresponse: def __init__(self, resp): self.json_data = resp + self.content = "github" def json(self): return [(self.json_data)] @@ -35,7 +36,7 @@ def test_slug_sub_stream_selected_slug_selected(self, mocked_team_members, mocke "breadcrumb": [ "properties", "name"], "metadata": {"inclusion": "available"} }] - tap_github.get_all_teams(schemas, "tap-github", {}, mdata) + tap_github.get_all_teams(schemas, "tap-github", {}, mdata, "") self.assertEquals(mocked_team_members.call_count, 1) @mock.patch("tap_github.__init__.get_all_team_members") @@ -58,7 +59,7 @@ def test_slug_sub_stream_not_selected_slug_selected(self, mocked_team_members, m "breadcrumb": [ "properties", "name"], "metadata": {"inclusion": "available"} }] - tap_github.get_all_teams(schemas, "tap-github", {}, mdata) + tap_github.get_all_teams(schemas, "tap-github", {}, mdata, "") self.assertEquals(mocked_team_members.call_count, 0) @mock.patch("tap_github.__init__.get_all_team_members") @@ -81,7 +82,7 @@ def test_slug_sub_stream_selected_slug_not_selected(self, mocked_team_members, m "breadcrumb": [ "properties", "name"], "metadata": {"inclusion": "available"} }] - tap_github.get_all_teams(schemas, "tap-github", {}, mdata) + tap_github.get_all_teams(schemas, "tap-github", {}, mdata, "") self.assertEquals(mocked_team_members.call_count, 1) @mock.patch("tap_github.__init__.get_all_team_members") @@ -104,7 +105,7 @@ def test_slug_sub_stream_not_selected_slug_not_selected(self, mocked_team_member "breadcrumb": [ "properties", "name"], "metadata": {"inclusion": "available"} }] - tap_github.get_all_teams(schemas, "tap-github", {}, mdata) + tap_github.get_all_teams(schemas, "tap-github", {}, mdata, "") self.assertEquals(mocked_team_members.call_count, 0) @mock.patch("tap_github.__init__.authed_get_all_pages") @@ -130,7 +131,7 @@ def test_user_not_selected_in_stargazers(self, mocked_write_records, mocked_requ "breadcrumb": ["properties", "starred_at"], "metadata": {"inclusion": "available"} }] - tap_github.get_all_stargazers(schemas, "tap-github", {}, mdata) + tap_github.get_all_stargazers(schemas, "tap-github", {}, mdata, "") self.assertEquals(mocked_write_records.call_count, 1) @mock.patch("singer.write_record") @@ -153,5 +154,5 @@ def test_user_selected_in_stargazers(self, mocked_write_records, mocked_request) "breadcrumb": ["properties", "starred_at"], "metadata": {"inclusion": "available"} }] - tap_github.get_all_stargazers(schemas, "tap-github", {}, mdata) + tap_github.get_all_stargazers(schemas, "tap-github", {}, mdata, "") self.assertEquals(mocked_write_records.call_count, 1) diff --git a/tests/unittests/test_stargazers_full_table.py b/tests/unittests/test_stargazers_full_table.py index b57e6393..47cb7089 100644 --- a/tests/unittests/test_stargazers_full_table.py +++ b/tests/unittests/test_stargazers_full_table.py @@ -9,6 +9,6 @@ def test_stargazers_without_query_params(self, mocked_request): schemas = {"stargazers": "None"} - tap_github.get_all_stargazers(schemas, "tap-github", {}, {}) + tap_github.get_all_stargazers(schemas, "tap-github", {}, {}, "") mocked_request.assert_called_with(mock.ANY, "https://api.github.com/repos/tap-github/stargazers", mock.ANY) diff --git a/tests/unittests/test_verify_access.py b/tests/unittests/test_verify_access.py index 59fb7e7e..4ed8ad7c 100644 --- a/tests/unittests/test_verify_access.py +++ b/tests/unittests/test_verify_access.py @@ -9,6 +9,7 @@ def __init__(self, status_code, json, raise_error, headers={'X-RateLimit-Remaini self.raise_error = raise_error self.text = json self.headers = headers + self.content = "github" def raise_for_status(self): if not self.raise_error: @@ -26,7 +27,7 @@ def get_response(status_code, json={}, raise_error=False): class TestCredentials(unittest.TestCase): def test_repo_not_found(self, mocked_request): - json = {"message": "Not Found", "documentation_url": "https:/"} + json = {"message": "Not Found", "documentation_url": "https:/docs.github.com/"} mocked_request.return_value = get_response(404, json, True) try: @@ -43,7 +44,7 @@ def test_repo_bad_request(self, mocked_request): self.assertEquals(str(e), "HTTP-error-code: 400, Error: The request is missing or has a bad parameter.") def test_repo_bad_creds(self, mocked_request): - json = {"message": "Bad credentials", "documentation_url": "https://docs.github.com/rest"} + json = {"message": "Bad credentials", "documentation_url": "https://docs.github.com/"} mocked_request.return_value = get_response(401, json, True) try: @@ -51,41 +52,6 @@ def test_repo_bad_creds(self, mocked_request): except tap_github.BadCredentialsException as e: self.assertEquals(str(e), "HTTP-error-code: 401, Error: {}".format(json)) - def test_org_not_found(self, mocked_request): - json = {"message": "Not Found", "documentation_url": "https:/"} - mocked_request.return_value = get_response(404, json, True) - - try: - tap_github.verify_org_access("", "personal-repo") - except tap_github.NotFoundException as e: - self.assertEquals(str(e), "HTTP-error-code: 404, Error: 'personal-repo' is not an organization.") - - def test_org_bad_request(self, mocked_request): - mocked_request.return_value = get_response(400, raise_error = True) - - try: - tap_github.verify_org_access("", "personal-repo") - except tap_github.BadRequestException as e: - self.assertEquals(str(e), "HTTP-error-code: 400, Error: The request is missing or has a bad parameter.") - - def test_org_forbidden(self, mocked_request): - json = {'message': 'Must have admin rights to Repository.', 'documentation_url': 'https://docs.github.com/rest/reference/'} - mocked_request.return_value = get_response(403, json, True) - - try: - tap_github.verify_org_access("", "personal-repo") - except tap_github.AuthException as e: - self.assertEquals(str(e), "HTTP-error-code: 403, Error: {}".format(json)) - - def test_org_bad_creds(self, mocked_request): - json = {"message": "Bad credentials", "documentation_url": "https://docs.github.com/rest"} - mocked_request.return_value = get_response(401, json, True) - - try: - tap_github.verify_org_access("", "personal-repo") - except tap_github.BadCredentialsException as e: - self.assertEquals(str(e), "HTTP-error-code: 401, Error: {}".format(json)) - @mock.patch("tap_github.get_catalog") def test_discover_valid_creds(self, mocked_get_catalog, mocked_request): mocked_request.return_value = get_response(200) @@ -97,7 +63,7 @@ def test_discover_valid_creds(self, mocked_get_catalog, mocked_request): @mock.patch("tap_github.get_catalog") def test_discover_not_found(self, mocked_get_catalog, mocked_request): - json = {"message": "Not Found", "documentation_url": "https:/"} + json = {"message": "Not Found", "documentation_url": "https:/docs.github.com/"} mocked_request.return_value = get_response(404, json, True) mocked_get_catalog.return_value = {} @@ -120,7 +86,7 @@ def test_discover_bad_request(self, mocked_get_catalog, mocked_request): @mock.patch("tap_github.get_catalog") def test_discover_bad_creds(self, mocked_get_catalog, mocked_request): - json = {"message":"Bad credentials","documentation_url":"https://docs.github.com/rest"} + json = {"message":"Bad credentials","documentation_url":"https://docs.github.com/"} mocked_request.return_value = get_response(401, json, True) mocked_get_catalog.return_value = {} @@ -132,7 +98,7 @@ def test_discover_bad_creds(self, mocked_get_catalog, mocked_request): @mock.patch("tap_github.get_catalog") def test_discover_forbidden(self, mocked_get_catalog, mocked_request): - json = {'message': 'Must have admin rights to Repository.', 'documentation_url': 'https://docs.github.com/rest/reference/'} + json = {'message': 'Must have admin rights to Repository.', 'documentation_url': 'https://docs.github.com/'} mocked_request.return_value = get_response(403, json, True) mocked_get_catalog.return_value = {} @@ -145,19 +111,16 @@ def test_discover_forbidden(self, mocked_get_catalog, mocked_request): @mock.patch("tap_github.logger.info") @mock.patch("tap_github.verify_repo_access") -@mock.patch("tap_github.verify_org_access") class TestRepoCallCount(unittest.TestCase): - def test_repo_call_count(self, mocked_org, mocked_repo, mocked_logger_info): + def test_repo_call_count(self, mocked_repo, mocked_logger_info): """ Here 3 repos are given, so tap will check creds for all 3 repos """ - mocked_org.return_value = None mocked_repo.return_value = None config = {"access_token": "access_token", "repository": "org1/repo1 org1/repo2 org2/repo1"} - tap_github.verify_access_for_repo_org(config) + tap_github.verify_access_for_repo(config) self.assertEquals(mocked_logger_info.call_count, 3) - self.assertEquals(mocked_org.call_count, 3) self.assertEquals(mocked_repo.call_count, 3)