Skip to content

Commit 1ffdf0d

Browse files
stray-nickpi-monopre-commit-ci[bot]
authored
fix: support repositories with pull requests disabled (MeltanoLabs#571)
## Summary This PR lets the tap handle repositories that have pull requests disabled. GitHub can return `"has_pull_requests": false` in a repository payload. For those repositories, `GET /repos/{owner}/{repo}/pulls` can return `404 Not Found` even though the repository itself is readable and other endpoints can still be extracted. Instead of failing the sync for that case, the tap now skips pull request extraction for repositories where GitHub explicitly reports that pull requests are disabled. ## Behavior - When `has_pull_requests` is exactly `false`, the tap skips the `/pulls` request for that repository. - When `has_pull_requests` is `true`, missing, `null`, `0`, or otherwise unknown, behavior is unchanged: the tap still requests `/pulls` and surfaces GitHub errors normally. - The change applies only to pull request extraction. ## Implementation - Includes GitHub's `has_pull_requests` field in repository records. - Carries that flag to the pull request extraction step. - Checks the flag before requesting `/repos/{owner}/{repo}/pulls`. ## What this does not change - It does not add broad `404` or `403` tolerance. - It does not change issue extraction or infer pull request availability from `has_issues`. - It does not change discussion extraction. ## Validation ```text LOGLEVEL=WARNING uv run pytest tests/test_tap.py -k 'pull_request_capability or pull_requests_stream or issues_stream' # 9 passed, 8 deselected uv run ruff check tap_github/repository_streams.py tests/test_tap.py # All checks passed uv run mypy tap_github # Success: no issues found in 12 source files uv run ty check tap_github # All checks passed git diff --check # passed ``` <!-- github-gate:idempotency=tap-github-pr571-body-proposal-only --> --------- Co-authored-by: AI (Pi) <noreply@pi.dev> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
1 parent e2abbee commit 1ffdf0d

2 files changed

Lines changed: 154 additions & 0 deletions

File tree

tap_github/repository_streams.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,7 @@ def get_child_context(self, record: dict, context: Context | None) -> dict:
243243
"has_discussions": record.get(
244244
"has_discussions", False
245245
), # GitHub repos not updated after the feature was released in 2021 will not have this field. # noqa: E501
246+
"has_pull_requests": record.get("has_pull_requests", True),
246247
}
247248

248249
def get_records(self, context: Context | None) -> Iterable[dict[str, Any]]:
@@ -324,6 +325,7 @@ def get_records(self, context: Context | None) -> Iterable[dict[str, Any]]:
324325
th.Property("network_count", th.IntegerType),
325326
th.Property("subscribers_count", th.IntegerType),
326327
th.Property("open_issues_count", th.IntegerType),
328+
th.Property("has_pull_requests", th.BooleanType),
327329
th.Property("allow_squash_merge", th.BooleanType),
328330
th.Property("allow_merge_commit", th.BooleanType),
329331
th.Property("allow_rebase_merge", th.BooleanType),
@@ -1302,6 +1304,24 @@ class PullRequestsStream(GitHubRestStream):
13021304
# GitHub is missing the "since" parameter on this endpoint.
13031305
use_fake_since_parameter = True
13041306

1307+
def get_records(self, context: Context | None = None) -> Iterable[dict[str, Any]]:
1308+
"""
1309+
Return a generator of row-type dictionary objects.
1310+
If pull requests are not enabled, skip the API call.
1311+
"""
1312+
assert context is not None, f"Context cannot be empty for '{self.name}' stream"
1313+
1314+
repo = context.get("repo", "unknown")
1315+
org = context.get("org", "unknown")
1316+
if context.get("has_pull_requests", True) is False:
1317+
self.logger.debug(
1318+
f"Repository {org}/{repo}: Pull requests not enabled, "
1319+
"skipping API call",
1320+
)
1321+
return []
1322+
1323+
return super().get_records(context)
1324+
13051325
def get_url_params(
13061326
self,
13071327
context: Context | None,

tests/test_tap.py

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,12 @@
77
import pytest
88
from bs4 import BeautifulSoup
99
from dateutil.parser import isoparse
10+
from requests import Response
11+
from singer_sdk.exceptions import RetriableAPIError
1012
from singer_sdk.helpers import _catalog as cat_helpers
1113
from singer_sdk.singerlib import Catalog
1214

15+
from tap_github.repository_streams import GitHubRestStream
1316
from tap_github.scraping import parse_counter
1417
from tap_github.tap import TapGitHub
1518

@@ -126,6 +129,137 @@ def test_get_a_repository_in_repo_list_mode(
126129
assert '"org": "meltanolabs"' not in captured_out
127130

128131

132+
def test_repository_child_context_includes_pull_request_capability(repo_list_config): # noqa: F811
133+
"""Verify repository context includes the pull request capability flag."""
134+
tap = TapGitHub(config=repo_list_config)
135+
repository_stream = tap.streams["repositories"]
136+
137+
context = repository_stream.get_child_context(
138+
{
139+
"id": 123,
140+
"name": "issues-pi",
141+
"owner": {"login": "shop"},
142+
"has_pull_requests": False,
143+
},
144+
None,
145+
)
146+
147+
assert context["has_pull_requests"] is False
148+
149+
150+
def test_repository_child_context_defaults_pull_request_capability_to_enabled(
151+
repo_list_config, # noqa: F811
152+
):
153+
"""Preserve existing behavior when GitHub does not return the capability flag."""
154+
tap = TapGitHub(config=repo_list_config)
155+
repository_stream = tap.streams["repositories"]
156+
157+
context = repository_stream.get_child_context(
158+
{
159+
"id": 123,
160+
"name": "tap-github",
161+
"owner": {"login": "MeltanoLabs"},
162+
},
163+
None,
164+
)
165+
166+
assert context["has_pull_requests"] is True
167+
168+
169+
def test_pull_requests_stream_skips_repos_with_pull_requests_disabled(
170+
repo_list_config, # noqa: F811
171+
):
172+
"""Do not call the pull requests API when repo metadata says it is disabled."""
173+
tap = TapGitHub(config=repo_list_config)
174+
pull_requests_stream = tap.streams["pull_requests"]
175+
context = {
176+
"org": "shop",
177+
"repo": "issues-pi",
178+
"repo_id": 123,
179+
"has_pull_requests": False,
180+
}
181+
182+
with (
183+
patch.object(GitHubRestStream, "get_records") as get_records,
184+
patch.object(pull_requests_stream.logger, "debug") as log_debug,
185+
):
186+
records = list(pull_requests_stream.get_records(context))
187+
188+
assert records == []
189+
get_records.assert_not_called()
190+
log_debug.assert_called_once_with(
191+
"Repository shop/issues-pi: Pull requests not enabled, skipping API call",
192+
)
193+
194+
195+
@pytest.mark.parametrize("has_pull_requests", [True, None, 0, "missing"])
196+
def test_pull_requests_stream_delegates_when_pull_request_capability_is_not_false(
197+
repo_list_config, # noqa: F811
198+
has_pull_requests,
199+
):
200+
"""Fail open when the capability flag is enabled, missing, or unknown."""
201+
tap = TapGitHub(config=repo_list_config)
202+
pull_requests_stream = tap.streams["pull_requests"]
203+
context = {
204+
"org": "MeltanoLabs",
205+
"repo": "tap-github",
206+
"repo_id": 123,
207+
}
208+
if has_pull_requests != "missing":
209+
context["has_pull_requests"] = has_pull_requests
210+
211+
with patch.object(
212+
GitHubRestStream,
213+
"get_records",
214+
return_value=iter([{"id": 456}]),
215+
) as get_records:
216+
records = list(pull_requests_stream.get_records(context))
217+
218+
assert records == [{"id": 456}]
219+
get_records.assert_called_once_with(context)
220+
221+
222+
def test_issues_stream_delegates_when_has_issues_is_false(
223+
repo_list_config, # noqa: F811
224+
):
225+
"""Do not infer issues gating from repository feature metadata."""
226+
tap = TapGitHub(config=repo_list_config)
227+
issues_stream = tap.streams["issues"]
228+
context = {
229+
"org": "shop",
230+
"repo": "issues-pi",
231+
"repo_id": 123,
232+
"has_issues": False,
233+
}
234+
235+
with patch.object(
236+
GitHubRestStream,
237+
"get_records",
238+
return_value=iter([{"id": 789}]),
239+
) as get_records:
240+
records = list(issues_stream.get_records(context))
241+
242+
assert records == [{"id": 789}]
243+
get_records.assert_called_once_with(context)
244+
245+
246+
def test_pull_requests_stream_keeps_generic_404_retriable(
247+
repo_list_config, # noqa: F811
248+
):
249+
"""Pull request gating must not broaden tolerated GitHub errors."""
250+
tap = TapGitHub(config=repo_list_config)
251+
pull_requests_stream = tap.streams["pull_requests"]
252+
response = Response()
253+
response.status_code = 404
254+
response.url = "https://api.github.com/repos/shop/issues-pi/pulls"
255+
response.reason = "Not Found"
256+
response._content = b'{"message": "Not Found"}'
257+
response.headers["X-GitHub-Request-Id"] = "request-id"
258+
259+
with pytest.raises(RetriableAPIError, match="404 Client Error"):
260+
pull_requests_stream.validate_response(response)
261+
262+
129263
@pytest.mark.repo_list(["MeltanoLabs/tap-github"])
130264
def test_last_state_message_is_valid(capsys, repo_list_config): # noqa: F811
131265
"""

0 commit comments

Comments
 (0)