From 66fc48d95b893e506e4d4e97638dba98d66c4a8e Mon Sep 17 00:00:00 2001 From: John Comar Date: Mon, 15 Jun 2026 10:13:01 -0400 Subject: [PATCH] fix: skip unavailable issue comments endpoint --- tap_github/repository_streams.py | 15 ++++ tests/test_tap.py | 141 +++++++++++++++++++++++++++++-- 2 files changed, 150 insertions(+), 6 deletions(-) diff --git a/tap_github/repository_streams.py b/tap_github/repository_streams.py index 613d2ea5..52348dcb 100644 --- a/tap_github/repository_streams.py +++ b/tap_github/repository_streams.py @@ -243,6 +243,7 @@ def get_child_context(self, record: dict, context: Context | None) -> dict: "has_discussions": record.get( "has_discussions", False ), # GitHub repos not updated after the feature was released in 2021 will not have this field. # noqa: E501 + "has_issues": record.get("has_issues", True), "has_pull_requests": record.get("has_pull_requests", True), } @@ -325,6 +326,7 @@ def get_records(self, context: Context | None) -> Iterable[dict[str, Any]]: th.Property("network_count", th.IntegerType), th.Property("subscribers_count", th.IntegerType), th.Property("open_issues_count", th.IntegerType), + th.Property("has_issues", th.BooleanType), th.Property("has_pull_requests", th.BooleanType), th.Property("allow_squash_merge", th.BooleanType), th.Property("allow_merge_commit", th.BooleanType), @@ -1018,6 +1020,19 @@ def get_records(self, context: Context | None = None) -> Iterable[dict[str, Any] Each row emitted should be a dictionary of property names to their values. """ + if ( + context + and context.get("has_issues", True) is False + and context.get("has_pull_requests", True) is False + ): + repo = context.get("repo", "unknown") + org = context.get("org", "unknown") + self.logger.debug( + f"Repository {org}/{repo}: Issues and pull requests not enabled, " + "skipping issue comments API call", + ) + return [] + if context and context.get("comments", None) == 0: self.logger.debug(f"No comments detected. Skipping '{self.name}' sync.") return [] diff --git a/tests/test_tap.py b/tests/test_tap.py index 37b82177..c0b25203 100644 --- a/tests/test_tap.py +++ b/tests/test_tap.py @@ -189,6 +189,43 @@ def test_repository_child_context_defaults_pull_request_capability_to_enabled( assert context["has_pull_requests"] is True +def test_repository_child_context_includes_issue_capability(repo_list_config): # noqa: F811 + """Verify repository context includes the issue capability flag.""" + tap = TapGitHub(config=repo_list_config) + repository_stream = tap.streams["repositories"] + + context = repository_stream.get_child_context( + { + "id": 123, + "name": "no-issues", + "owner": {"login": "octo-org"}, + "has_issues": False, + }, + None, + ) + + assert context["has_issues"] is False + + +def test_repository_child_context_defaults_issue_capability_to_enabled( + repo_list_config, # noqa: F811 +): + """Preserve existing behavior when GitHub does not return the issue flag.""" + tap = TapGitHub(config=repo_list_config) + repository_stream = tap.streams["repositories"] + + context = repository_stream.get_child_context( + { + "id": 123, + "name": "tap-github", + "owner": {"login": "MeltanoLabs"}, + }, + None, + ) + + assert context["has_issues"] is True + + def test_pull_requests_stream_skips_repos_with_pull_requests_disabled( repo_list_config, # noqa: F811 ): @@ -242,15 +279,90 @@ def test_pull_requests_stream_delegates_when_pull_request_capability_is_not_fals get_records.assert_called_once_with(context) -def test_issues_stream_delegates_when_has_issues_is_false( +def test_issue_comments_stream_skips_when_issues_and_pull_requests_are_disabled( repo_list_config, # noqa: F811 ): - """Do not infer issues gating from repository feature metadata.""" + """Do not call the issue comments API when repo metadata says it is disabled.""" tap = TapGitHub(config=repo_list_config) - issues_stream = tap.streams["issues"] + issue_comments_stream = tap.streams["issue_comments"] context = { - "org": "shop", - "repo": "issues-pi", + "org": "octo-org", + "repo": "no-issues-or-pull-requests", + "repo_id": 123, + "has_issues": False, + "has_pull_requests": False, + } + + with ( + patch.object(GitHubRestStream, "get_records") as get_records, + patch.object(issue_comments_stream.logger, "debug") as log_debug, + ): + records = list(issue_comments_stream.get_records(context)) + + assert records == [] + get_records.assert_not_called() + log_debug.assert_called_once_with( + "Repository octo-org/no-issues-or-pull-requests: " + "Issues and pull requests not enabled, skipping issue comments API call", + ) + + +@pytest.mark.parametrize( + ("has_issues", "has_pull_requests"), + [ + (True, False), + (None, False), + (0, False), + ("missing", False), + (False, True), + (False, None), + (False, 0), + (False, "missing"), + ], +) +def test_issue_comments_stream_delegates_when_capabilities_are_not_false( + repo_list_config, # noqa: F811 + has_issues, + has_pull_requests, +): + """Fail open when either capability flag is enabled, missing, or unknown.""" + tap = TapGitHub(config=repo_list_config) + issue_comments_stream = tap.streams["issue_comments"] + context = { + "org": "MeltanoLabs", + "repo": "tap-github", + "repo_id": 123, + } + if has_issues != "missing": + context["has_issues"] = has_issues + if has_pull_requests != "missing": + context["has_pull_requests"] = has_pull_requests + + with patch.object( + GitHubRestStream, + "get_records", + return_value=iter([{"id": 456}]), + ) as get_records: + records = list(issue_comments_stream.get_records(context)) + + assert records == [{"id": 456}] + get_records.assert_called_once_with(context) + + +@pytest.mark.parametrize( + "stream_name", + ["issues", "issue_events", "milestones", "labels"], +) +def test_issue_adjacent_streams_delegate_when_has_issues_is_false( + repo_list_config, # noqa: F811 + stream_name, +): + """Document streams intentionally not gated by the repository Issues flag.""" + tap = TapGitHub(config=repo_list_config) + stream = tap.streams[stream_name] + context = { + "org": "octo-org", + "repo": "no-issues", "repo_id": 123, "has_issues": False, } @@ -260,7 +372,7 @@ def test_issues_stream_delegates_when_has_issues_is_false( "get_records", return_value=iter([{"id": 789}]), ) as get_records: - records = list(issues_stream.get_records(context)) + records = list(stream.get_records(context)) assert records == [{"id": 789}] get_records.assert_called_once_with(context) @@ -283,6 +395,23 @@ def test_pull_requests_stream_keeps_generic_404_retriable( pull_requests_stream.validate_response(response) +def test_issue_comments_stream_keeps_generic_404_retriable( + repo_list_config, # noqa: F811 +): + """Issue comments gating must not broaden tolerated GitHub errors.""" + tap = TapGitHub(config=repo_list_config) + issue_comments_stream = tap.streams["issue_comments"] + response = Response() + response.status_code = 404 + response.url = "https://api.github.com/repos/octo-org/no-issues/issues/comments" + response.reason = "Not Found" + response._content = b'{"message": "Not Found"}' + response.headers["X-GitHub-Request-Id"] = "request-id" + + with pytest.raises(RetriableAPIError, match="404 Client Error"): + issue_comments_stream.validate_response(response) + + @pytest.mark.repo_list(["MeltanoLabs/tap-github"]) def test_last_state_message_is_valid(capsys, repo_list_config): # noqa: F811 """