From 2413e6f064daf407e1d2c316e58fbe27e816f831 Mon Sep 17 00:00:00 2001 From: Peter Kosztolanyi Date: Thu, 26 Aug 2021 09:19:01 +0200 Subject: [PATCH 1/3] Add verify_all_repo_access config parameter --- README.md | 4 +++- tap_github/__init__.py | 2 ++ tests/unittests/test_verify_access.py | 23 +++++++++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 91a5131..2f5d648 100644 --- a/README.md +++ b/README.md @@ -59,6 +59,7 @@ include_archived |no |true/false to include archived repos. D include_disabled |no |true/false to include disabled repos. Default false repository |no |(DEPRECATED) Allow list strategy to extract selected repos data from organization(has priority over repos_exclude) max_rate_limit_wait_seconds |no |Max time to wait if you hit the github api limit. DEFAULT to 600s +verify_all_repo_access |no |Verify access to all selected repositories. If it's false then it verifies access only to the first selected repository. Default true Example: ```json @@ -70,7 +71,8 @@ Example: "start_date": "2021-01-01T00:00:00Z", "include_archived": false, "include_disabled": false, - "max_rate_limit_wait_seconds": 800 + "max_rate_limit_wait_seconds": 800, + "verify_all_repo_access": true } ``` diff --git a/tap_github/__init__.py b/tap_github/__init__.py index 0a8e61b..ca8a289 100644 --- a/tap_github/__init__.py +++ b/tap_github/__init__.py @@ -349,6 +349,8 @@ def verify_access_for_repo(config): session.headers.update({'authorization': 'token ' + access_token, 'per_page': '1', 'page': '1'}) repositories = get_repos_from_config(config) + if not config.get('verify_all_repo_access', True) and len(repositories) > 0: + repositories = [repositories[0]] for repo in repositories: logger.info("Verifying access of repository: %s", repo) diff --git a/tests/unittests/test_verify_access.py b/tests/unittests/test_verify_access.py index be48c8f..fefc607 100644 --- a/tests/unittests/test_verify_access.py +++ b/tests/unittests/test_verify_access.py @@ -124,3 +124,26 @@ def test_repo_call_count(self, mocked_repo, mocked_logger_info): self.assertEqual(mocked_logger_info.call_count, 3) self.assertEqual(mocked_repo.call_count, 3) + + def test_verify_repo_access_call_count(self, mocked_repo, mocked_logger_info): + mocked_repo.return_value = None + + # Should verify all repository access by default + tap_github.verify_access_for_repo({"access_token": "access_token", + "repository": "org1/repo1 org1/repo2 org2/repo1"}) + self.assertEqual(mocked_repo.call_count, 3) + + # Should verify all repository access if verify_all_repo_access is False + mocked_repo.reset_mock() + tap_github.verify_access_for_repo({"access_token": "access_token", + "repository": "org1/repo1 org1/repo2 org2/repo1", + "verify_all_repo_access": False}) + self.assertEqual(mocked_repo.call_count, 1) + + # Should verify all repository access if verify_all_repo_access is True + mocked_repo.reset_mock() + tap_github.verify_access_for_repo({"access_token": "access_token", + "repository": "org1/repo1 org1/repo2 org2/repo1", + "verify_all_repo_access": True}) + self.assertEqual(mocked_repo.call_count, 3) + From 9250866665b4776bf0cf6c160a49df191dc3ad98 Mon Sep 17 00:00:00 2001 From: Peter Kosztolanyi Date: Thu, 26 Aug 2021 09:35:36 +0200 Subject: [PATCH 2/3] Fix comments --- tests/unittests/test_verify_access.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unittests/test_verify_access.py b/tests/unittests/test_verify_access.py index fefc607..40f8543 100644 --- a/tests/unittests/test_verify_access.py +++ b/tests/unittests/test_verify_access.py @@ -128,19 +128,19 @@ def test_repo_call_count(self, mocked_repo, mocked_logger_info): def test_verify_repo_access_call_count(self, mocked_repo, mocked_logger_info): mocked_repo.return_value = None - # Should verify all repository access by default + # Should verify all repository accesses by default tap_github.verify_access_for_repo({"access_token": "access_token", "repository": "org1/repo1 org1/repo2 org2/repo1"}) self.assertEqual(mocked_repo.call_count, 3) - # Should verify all repository access if verify_all_repo_access is False + # Should verify only one repository access if verify_all_repo_access is False mocked_repo.reset_mock() tap_github.verify_access_for_repo({"access_token": "access_token", "repository": "org1/repo1 org1/repo2 org2/repo1", "verify_all_repo_access": False}) self.assertEqual(mocked_repo.call_count, 1) - # Should verify all repository access if verify_all_repo_access is True + # Should verify all repository accesses if verify_all_repo_access is True mocked_repo.reset_mock() tap_github.verify_access_for_repo({"access_token": "access_token", "repository": "org1/repo1 org1/repo2 org2/repo1", From b4f9e5a2ba743ebce7e4a731580df3de0f885828 Mon Sep 17 00:00:00 2001 From: Peter Kosztolanyi Date: Thu, 26 Aug 2021 11:03:48 +0200 Subject: [PATCH 3/3] Don't mutate passed parameter --- tap_github/__init__.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tap_github/__init__.py b/tap_github/__init__.py index ca8a289..c57bb26 100644 --- a/tap_github/__init__.py +++ b/tap_github/__init__.py @@ -349,10 +349,16 @@ def verify_access_for_repo(config): session.headers.update({'authorization': 'token ' + access_token, 'per_page': '1', 'page': '1'}) repositories = get_repos_from_config(config) - if not config.get('verify_all_repo_access', True) and len(repositories) > 0: - repositories = [repositories[0]] + verify_all = config.get('verify_all_repo_access', True) - for repo in repositories: + if verify_all and len(repositories) > 0: + repos_to_verify = repositories + elif len(repositories) > 0: + repos_to_verify = [repositories[0]] + else: + repos_to_verify = [] + + for repo in repos_to_verify: logger.info("Verifying access of repository: %s", repo) url_for_repo = "https://api.github.com/repos/{}/commits".format(repo)