chore: Remove List Repos Generator Exp Control Path (#480)

codecov · May 31, 2024 · 2d39bca · 2d39bca
1 parent 4a0347d
commit 2d39bca
Show file tree

Hide file tree

Showing 7 changed files with 39 additions and 227 deletions.
diff --git a/helpers/backfills.py b/helpers/backfills.py
@@ -17,6 +17,7 @@ def add_repos_service_ids_from_provider(
     owner_service: torngit.base.TorngitBaseAdapter,
     gh_app_installation: GithubAppInstallation,
 ):
+    # TODO: Convert this to the generator function
     repos = async_to_sync(owner_service.list_repos_using_installation)()
 
     if repos:

diff --git a/rollouts/__init__.py b/rollouts/__init__.py
@@ -1,8 +1,6 @@
 from shared.rollouts import Feature
 
 # Declare the feature variants and parameters via Django Admin
-LIST_REPOS_GENERATOR_BY_OWNER_ID = Feature("list_repos_generator")
-
 FLAKY_TEST_DETECTION = Feature("flaky_test_detection")
 
 # Eventually we want all repos to use this

diff --git a/service_ids b/service_ids
diff --git a/tasks/sync_repos.py b/tasks/sync_repos.py
@@ -19,7 +19,6 @@
 
 from app import celery_app
 from database.models import Owner, Repository
-from rollouts import LIST_REPOS_GENERATOR_BY_OWNER_ID
 from services.owner import get_owner_provider_service
 from services.redis import get_redis_connection
 from tasks.base import BaseCodecovTask
@@ -318,29 +317,15 @@ def process_repos(repos):
                 db_session, git, owner, repository_service_ids
             )
             repoids = repoids_added
-        # The `if` below should be `elif` but we had issues recently
-        # related to the sync task when repos are known
+        # Below logic may not be needed if repository_service_ids exist, but
+        # we have run into issues related to the sync task when repos are known
         # So we should still run it just in case and possibly update GithubInstallation.repository_service_ids
         # Instead of relying exclusively on the webhooks to do that
         # TODO: Maybe we don't need to run this every time, but once in a while just in case...
-        if await LIST_REPOS_GENERATOR_BY_OWNER_ID.check_value_async(
-            identifier=ownerid, default=False
-        ):
-            with metrics.timer(
-                f"{metrics_scope}.sync_repos_using_integration.list_repos_generator"
-            ):
-                async for page in git.list_repos_using_installation_generator(username):
-                    if page:
-                        received_repos = True
-                        process_repos(page)
-        else:
-            with metrics.timer(
-                f"{metrics_scope}.sync_repos_using_integration.list_repos"
-            ):
-                repos = await git.list_repos_using_installation(username)
-            if repos:
+        async for page in git.list_repos_using_installation_generator(username):
+            if page:
                 received_repos = True
-                process_repos(repos)
+                process_repos(page)
 
         # If the installation returned no repos, we were probably disabled and
         # should indicate as much on this owner's repositories.
@@ -436,17 +421,9 @@ def process_repos(repos):
                     db_session.commit()
 
         try:
-            if await LIST_REPOS_GENERATOR_BY_OWNER_ID.check_value_async(
-                identifier=ownerid, default=False
-            ):
-                with metrics.timer(f"{metrics_scope}.sync_repos.list_repos_generator"):
-                    async for page in git.list_repos_generator():
-                        process_repos(page)
-            else:
-                # get my repos (and team repos)
-                with metrics.timer(f"{metrics_scope}.sync_repos.list_repos"):
-                    repos = await git.list_repos()
-                    process_repos(repos)
+            async for page in git.list_repos_generator():
+                process_repos(page)
+
         except SoftTimeLimitExceeded:
             old_permissions = owner.permission or []
             log.warning(

diff --git a/tasks/tests/unit/test_backfill_existing_gh_app_installations.py b/tasks/tests/unit/test_backfill_existing_gh_app_installations.py
@@ -165,7 +165,7 @@ def test_gh_app_without_all_repo_selection(
         }
         mock_repo_provider.list_repos_using_installation.return_value = mock_repos
         mocker.patch(
-            f"tasks.backfill_existing_gh_app_installations.get_owner_provider_service",
+            "tasks.backfill_existing_gh_app_installations.get_owner_provider_service",
             return_value=mock_repo_provider,
         )
 

diff --git a/tasks/tests/unit/test_backfill_owners_without_gh_app_installations.py b/tasks/tests/unit/test_backfill_owners_without_gh_app_installations.py
@@ -54,7 +54,7 @@ def test_no_previous_app_existing_repos_only(
         # Mock fn return values
         mock_repo_provider.list_repos_using_installation.return_value = mock_repos
         mocker.patch(
-            f"tasks.backfill_owners_without_gh_app_installations.get_owner_provider_service",
+            "tasks.backfill_owners_without_gh_app_installations.get_owner_provider_service",
             return_value=mock_repo_provider,
         )
 
@@ -102,7 +102,7 @@ def test_no_previous_app_some_existing_repos(
         # Mock fn return values
         mock_repo_provider.list_repos_using_installation.return_value = mock_repos
         mocker.patch(
-            f"tasks.backfill_owners_without_gh_app_installations.get_owner_provider_service",
+            "tasks.backfill_owners_without_gh_app_installations.get_owner_provider_service",
             return_value=mock_repo_provider,
         )