ref(github): Use real GitHub API pagination instead of cached list slicing

Paginated repo browsing now makes a single GitHub API call per page
via GET /installation/repositories?page=N&per_page=M instead of
fetching all repos into a cache and slicing. This avoids loading
the full repo list into memory on every request.

Also switches to the built-in get_per_page() and
get_cursor_from_request() helpers, raises NotImplementedError in the
base class instead of returning None, and removes the custom
_parse_cursor method.

Refs VDY-46

Author: jaydgoss

src/sentry/integrations/api/endpoints/organization_integration_repos.py

@@ -85,22 +85,19 @@ def get(
             # continue to receive the full list.
             paginate = "per_page" in request.GET and not search
             if paginate:
+                per_page = self.get_per_page(request)
+                cursor = self.get_cursor_from_request(request)
+                offset = max(0, cursor.offset) if cursor is not None else 0
                 try:
-                    per_page = max(1, min(int(request.GET["per_page"]), 100))
-                except (ValueError, TypeError):
-                    per_page = 100
-                cursor = self._parse_cursor(request)
-                offset = max(0, cursor.offset)
-                try:
-                    paginated = install.get_repositories_paginated(offset=offset, per_page=per_page)
+                    repositories, has_next = install.get_repositories_paginated(
+                        offset=offset, per_page=per_page
+                    )
+                except NotImplementedError:
+                    paginate = False
                 except (IntegrationError, IdentityNotValid) as e:
                     return self.respond({"detail": str(e)}, status=400)
-            else:
-                paginated = None
 
-            if paginated is not None:
-                repositories, has_next = paginated
-            else:
+            if not paginate:
                 try:
                     repositories = install.get_repositories(
                         search,
@@ -113,11 +110,6 @@ def get(
 
             installable_only = request.GET.get("installableOnly", "false").lower() == "true"
 
-            # NOTE: installableOnly filtering happens after pagination slicing,
-            # so pages may contain fewer items than per_page when installed repos
-            # are filtered out. This is acceptable for the infinite-scroll use
-            # case (SCM onboarding) but would need to move before slicing if
-            # exact page sizes matter for a future consumer.
             serialized_repositories = [
                 IntegrationRepository(
                     name=repo["name"],
@@ -133,9 +125,7 @@ def get(
                 {"repos": serialized_repositories, "searchable": install.repo_search}
             )
 
-            # per_page and offset are guaranteed set when paginated is not None
-            if paginated is not None and (has_next or offset > 0):
-                # CursorResult only used for Link header generation
+            if paginate and (has_next or offset > 0):
                 cursor_result: CursorResult[IntegrationRepository] = CursorResult(
                     results=[],
                     prev=Cursor(0, max(0, offset - per_page), True, offset > 0),
@@ -146,13 +136,3 @@ def get(
             return response
 
         return self.respond({"detail": "Repositories not supported"}, status=400)
-
-    @staticmethod
-    def _parse_cursor(request: Request) -> Cursor:
-        cursor_param = request.GET.get("cursor", "")
-        if not cursor_param:
-            return Cursor(0, 0, False)
-        try:
-            return Cursor.from_string(cursor_param)
-        except (ValueError, TypeError):
-            return Cursor(0, 0, False)

src/sentry/integrations/github/client.py

@@ -558,6 +558,26 @@ def get_repos(self, page_number_limit: int | None = None) -> list[dict[str, Any]
                 page_number_limit=page_number_limit,
             )
 
+    def get_repos_page(
+        self, page: int = 1, per_page: int = 100
+    ) -> tuple[list[dict[str, Any]], int]:
+        """
+        Fetch a single page of repositories accessible to this installation.
+
+        Returns (repositories, total_count).
+        https://docs.github.com/en/rest/apps/installations#list-repositories-accessible-to-the-app-installation
+        """
+        with SCMIntegrationInteractionEvent(
+            interaction_type=SCMIntegrationInteractionType.GET_REPOSITORIES,
+            provider_key=self.integration_name,
+            integration_id=self.integration.id,
+        ).capture():
+            response = self.get(
+                "/installation/repositories",
+                params={"per_page": per_page, "page": page},
+            )
+            return response["repositories"], response["total_count"]
+
     def get_repos_cached(self, ttl: int = 300) -> list[CachedRepo]:
         """
         Return all repos accessible to this installation, cached in

src/sentry/integrations/github/integration.py

@@ -380,21 +380,17 @@ def get_repositories_paginated(
         offset: int = 0,
         per_page: int = 100,
     ) -> tuple[list[RepositoryInfo], bool]:
-        """Paginate over the cached accessible-repos list.
+        """Fetch a single page of repos from the GitHub API.
 
-        Always serves from ``get_repos_cached()`` so every page is
-        fast after the initial cache warm-up. If the cache expires
-        between page fetches the underlying list can change,
-        which may cause duplicates or skipped repos across pages.
-        This is acceptable for infinite-scroll consumers.
+        Converts the cursor offset to a GitHub page number and makes
+        one API call per page request.
         """
         client = self.get_client()
-        all_repos = client.get_repos_cached()
-        repos = [r for r in all_repos if not r.get("archived")]
-
-        page = repos[offset : offset + per_page]
-        has_next = len(repos) > offset + per_page
-        return self._to_repo_info(page), has_next
+        page_number = (offset // per_page) + 1
+        repos, total_count = client.get_repos_page(page=page_number, per_page=per_page)
+        active_repos = [r for r in repos if not r.get("archived")]
+        has_next = (page_number * per_page) < total_count
+        return self._to_repo_info(active_repos), has_next
 
     def get_unmigratable_repositories(self) -> list[RpcRepository]:
         accessible_repos = self.get_repositories()

src/sentry/integrations/source_code_management/repository.py

@@ -91,16 +91,16 @@ def get_repositories_paginated(
         self,
         offset: int = 0,
         per_page: int = 100,
-    ) -> tuple[list[RepositoryInfo], bool] | None:
+    ) -> tuple[list[RepositoryInfo], bool]:
         """
         Return a page of repositories and whether more pages exist.
 
-        Returns ``(repos, has_next)`` for providers that support
-        paginated browsing, or ``None`` if the provider does not
-        implement pagination (callers should fall back to
-        ``get_repositories()``).
+        Returns ``(repos, has_next)``. Providers that don't support
+        paginated browsing should leave this unimplemented; the
+        endpoint catches ``NotImplementedError`` and falls back to
+        ``get_repositories()``.
         """
-        return None
+        raise NotImplementedError
 
 
 ClientT = TypeVar("ClientT", bound="RepositoryClient", default="RepositoryClient")

tests/sentry/integrations/api/endpoints/test_organization_integration_repos.py

@@ -1,16 +1,16 @@
+from typing import Any
 from unittest.mock import MagicMock, patch
 
-from sentry.integrations.github.client import CachedRepo
 from sentry.testutils.cases import APITestCase
 
 
-def _make_cached_repo(
+def _make_github_repo(
     id: int,
     name: str,
     full_name: str,
     default_branch: str | None = "main",
     archived: bool = False,
-) -> CachedRepo:
+) -> dict[str, Any]:
     return {
         "id": id,
         "name": name,
@@ -289,11 +289,13 @@ def test_no_repository_method(self) -> None:
         assert response.status_code == 400
 
 
-CACHED_REPOS = [_make_cached_repo(i, f"repo-{i}", f"Example/repo-{i}") for i in range(1, 6)]
+ALL_REPOS = [_make_github_repo(i, f"repo-{i}", f"Example/repo-{i}") for i in range(1, 6)]
+TOTAL_COUNT = len(ALL_REPOS)
+MOCK_TARGET = "sentry.integrations.github.client.GitHubBaseClient.get_repos_page"
 
 
 class OrganizationIntegrationReposPaginatedTest(APITestCase):
-    """Tests for cursor-based pagination triggered by sending per_page."""
+    """Tests for cursor-based pagination via GitHub API page params."""
 
     def setUp(self) -> None:
         super().setUp()
@@ -307,14 +309,13 @@ def setUp(self) -> None:
             f"/api/0/organizations/{self.org.slug}/integrations/{self.integration.id}/repos/"
         )
 
-    @patch(
-        "sentry.integrations.github.client.GitHubBaseClient.get_repos_cached",
-    )
-    def test_first_page(self, mock_cache: MagicMock) -> None:
-        mock_cache.return_value = CACHED_REPOS
+    @patch(MOCK_TARGET)
+    def test_first_page(self, mock_page: MagicMock) -> None:
+        mock_page.return_value = (ALL_REPOS[:2], TOTAL_COUNT)
         response = self.client.get(self.path, data={"per_page": "2"}, format="json")
 
         assert response.status_code == 200, response.content
+        mock_page.assert_called_once_with(page=1, per_page=2)
         repos = response.data["repos"]
         assert len(repos) == 2
         assert repos[0]["identifier"] == "Example/repo-1"
@@ -323,61 +324,61 @@ def test_first_page(self, mock_cache: MagicMock) -> None:
         assert 'rel="next"' in response["Link"]
         assert 'results="true"' in response["Link"].split("next")[1]
 
-    @patch(
-        "sentry.integrations.github.client.GitHubBaseClient.get_repos_cached",
-    )
-    def test_second_page(self, mock_cache: MagicMock) -> None:
-        mock_cache.return_value = CACHED_REPOS
+    @patch(MOCK_TARGET)
+    def test_second_page(self, mock_page: MagicMock) -> None:
+        mock_page.return_value = (ALL_REPOS[2:4], TOTAL_COUNT)
         response = self.client.get(
             self.path, data={"per_page": "2", "cursor": "0:2:0"}, format="json"
         )
 
         assert response.status_code == 200, response.content
+        mock_page.assert_called_once_with(page=2, per_page=2)
         repos = response.data["repos"]
         assert len(repos) == 2
         assert repos[0]["identifier"] == "Example/repo-3"
         assert repos[1]["identifier"] == "Example/repo-4"
 
-    @patch(
-        "sentry.integrations.github.client.GitHubBaseClient.get_repos_cached",
-    )
-    def test_last_page(self, mock_cache: MagicMock) -> None:
-        mock_cache.return_value = CACHED_REPOS
+    @patch(MOCK_TARGET)
+    def test_last_page(self, mock_page: MagicMock) -> None:
+        mock_page.return_value = (ALL_REPOS[4:], TOTAL_COUNT)
         response = self.client.get(
             self.path, data={"per_page": "2", "cursor": "0:4:0"}, format="json"
         )
 
         assert response.status_code == 200, response.content
+        mock_page.assert_called_once_with(page=3, per_page=2)
         repos = response.data["repos"]
         assert len(repos) == 1
         assert repos[0]["identifier"] == "Example/repo-5"
         link = response["Link"]
         next_part = link.split("next")[1]
         assert 'results="false"' in next_part
 
-    @patch(
-        "sentry.integrations.github.client.GitHubBaseClient.get_repos_cached",
-    )
-    def test_excludes_archived(self, mock_cache: MagicMock) -> None:
-        mock_cache.return_value = [
-            _make_cached_repo(1, "active", "Example/active"),
-            _make_cached_repo(2, "archived", "Example/archived", archived=True),
-        ]
+    @patch(MOCK_TARGET)
+    def test_excludes_archived(self, mock_page: MagicMock) -> None:
+        mock_page.return_value = (
+            [
+                _make_github_repo(1, "active", "Example/active"),
+                _make_github_repo(2, "archived", "Example/archived", archived=True),
+            ],
+            2,
+        )
         response = self.client.get(self.path, data={"per_page": "100"}, format="json")
 
         assert response.status_code == 200, response.content
         repos = response.data["repos"]
         assert len(repos) == 1
         assert repos[0]["identifier"] == "Example/active"
 
-    @patch(
-        "sentry.integrations.github.client.GitHubBaseClient.get_repos_cached",
-    )
-    def test_installable_only(self, mock_cache: MagicMock) -> None:
-        mock_cache.return_value = [
-            _make_cached_repo(1, "installed-repo", "Example/installed-repo"),
-            _make_cached_repo(2, "available-repo", "Example/available-repo"),
-        ]
+    @patch(MOCK_TARGET)
+    def test_installable_only(self, mock_page: MagicMock) -> None:
+        mock_page.return_value = (
+            [
+                _make_github_repo(1, "installed-repo", "Example/installed-repo"),
+                _make_github_repo(2, "available-repo", "Example/available-repo"),
+            ],
+            2,
+        )
         self.create_repo(
             project=self.project,
             integration_id=self.integration.id,
@@ -393,12 +394,10 @@ def test_installable_only(self, mock_cache: MagicMock) -> None:
         assert repos[0]["identifier"] == "Example/available-repo"
         assert repos[0]["isInstalled"] is False
 
-    @patch(
-        "sentry.integrations.github.client.GitHubBaseClient.get_repos_cached",
-    )
-    def test_no_cursor_on_single_page(self, mock_cache: MagicMock) -> None:
+    @patch(MOCK_TARGET)
+    def test_no_cursor_on_single_page(self, mock_page: MagicMock) -> None:
         """When all repos fit in one page, no Link header is added."""
-        mock_cache.return_value = [_make_cached_repo(1, "repo-1", "Example/repo-1")]
+        mock_page.return_value = ([_make_github_repo(1, "repo-1", "Example/repo-1")], 1)
         response = self.client.get(self.path, data={"per_page": "100"}, format="json")
 
         assert response.status_code == 200, response.content
@@ -434,67 +433,30 @@ def test_search_with_per_page_uses_full_list(self, get_repositories: MagicMock)
         get_repositories.assert_called_once_with("repo", accessible_only=False, use_cache=False)
         assert "Link" not in response
 
-    @patch(
-        "sentry.integrations.github.client.GitHubBaseClient.get_repos_cached",
-    )
-    def test_per_page_zero_clamped_to_one(self, mock_cache: MagicMock) -> None:
-        mock_cache.return_value = CACHED_REPOS
-        response = self.client.get(self.path, data={"per_page": "0"}, format="json")
+    def test_per_page_invalid_returns_400(self) -> None:
+        """Invalid per_page values (0, negative, non-numeric, >100) return 400."""
+        for value in ("0", "-1", "abc", "200"):
+            response = self.client.get(self.path, data={"per_page": value}, format="json")
+            assert response.status_code == 400, f"per_page={value} should return 400"
 
-        assert response.status_code == 200, response.content
-        assert len(response.data["repos"]) == 1
-
-    @patch(
-        "sentry.integrations.github.client.GitHubBaseClient.get_repos_cached",
-    )
-    def test_per_page_negative_clamped_to_one(self, mock_cache: MagicMock) -> None:
-        mock_cache.return_value = CACHED_REPOS
-        response = self.client.get(self.path, data={"per_page": "-1"}, format="json")
-
-        assert response.status_code == 200, response.content
-        assert len(response.data["repos"]) == 1
-
-    @patch(
-        "sentry.integrations.github.client.GitHubBaseClient.get_repos_cached",
-    )
-    def test_per_page_non_numeric_defaults_to_100(self, mock_cache: MagicMock) -> None:
-        mock_cache.return_value = CACHED_REPOS
-        response = self.client.get(self.path, data={"per_page": "abc"}, format="json")
-
-        assert response.status_code == 200, response.content
-        assert len(response.data["repos"]) == 5
-
-    @patch(
-        "sentry.integrations.github.client.GitHubBaseClient.get_repos_cached",
-    )
-    def test_per_page_over_max_clamped_to_100(self, mock_cache: MagicMock) -> None:
-        mock_cache.return_value = CACHED_REPOS
-        response = self.client.get(self.path, data={"per_page": "200"}, format="json")
-
-        assert response.status_code == 200, response.content
-        assert len(response.data["repos"]) == 5
-
-    @patch(
-        "sentry.integrations.github.client.GitHubBaseClient.get_repos_cached",
-    )
-    def test_negative_cursor_offset_clamped_to_zero(self, mock_cache: MagicMock) -> None:
-        mock_cache.return_value = CACHED_REPOS
+    @patch(MOCK_TARGET)
+    def test_negative_cursor_offset_clamped_to_zero(self, mock_page: MagicMock) -> None:
+        mock_page.return_value = (ALL_REPOS[:2], TOTAL_COUNT)
         response = self.client.get(
             self.path, data={"per_page": "2", "cursor": "0:-5:0"}, format="json"
         )
 
         assert response.status_code == 200, response.content
+        mock_page.assert_called_once_with(page=1, per_page=2)
         repos = response.data["repos"]
         assert len(repos) == 2
         assert repos[0]["identifier"] == "Example/repo-1"
 
-    @patch(
-        "sentry.integrations.github.client.GitHubBaseClient.get_repos_cached",
-    )
-    def test_integration_error_returns_400(self, mock_cache: MagicMock) -> None:
+    @patch(MOCK_TARGET)
+    def test_integration_error_returns_400(self, mock_page: MagicMock) -> None:
         from sentry.shared_integrations.exceptions import IntegrationError
 
-        mock_cache.side_effect = IntegrationError("token revoked")
+        mock_page.side_effect = IntegrationError("token revoked")
         response = self.client.get(self.path, data={"per_page": "2"}, format="json")
 
         assert response.status_code == 400