Repo sync: don't keep all objects in memory (#12404)

We were keeping all remote repos and org objects into memory, but only
using its remote_id, this should help to reduce the memory footprint
when syncing users with lots of repos.

Author: stsewd

readthedocs/oauth/services/base.py

@@ -255,16 +255,15 @@ def sync(self):
         - deletes old RemoteRepository/Organization that are not present
           for this user in the current provider
         """
-        remote_repositories = self.sync_repositories()
+        repository_remote_ids = self.sync_repositories()
         (
-            remote_organizations,
-            remote_repositories_organizations,
+            organization_remote_ids,
+            organization_repositories_remote_ids,
         ) = self.sync_organizations()
 
         # Delete RemoteRepository where the user doesn't have access anymore
         # (skip RemoteRepository tied to a Project on this user)
-        all_remote_repositories = remote_repositories + remote_repositories_organizations
-        repository_remote_ids = [r.remote_id for r in all_remote_repositories if r is not None]
+        repository_remote_ids += organization_repositories_remote_ids
         (
             self.user.remote_repository_relations.filter(
                 account=self.account,
@@ -277,7 +276,6 @@ def sync(self):
         )
 
         # Delete RemoteOrganization where the user doesn't have access anymore
-        organization_remote_ids = [o.remote_id for o in remote_organizations if o is not None]
         (
             self.user.remote_organization_relations.filter(
                 account=self.account,

readthedocs/oauth/services/bitbucket.py

@@ -34,7 +34,7 @@ class BitbucketService(UserService):
 
     def sync_repositories(self):
         """Sync repositories from Bitbucket API."""
-        remote_repositories = []
+        remote_ids = []
 
         # Get user repos
         try:
@@ -44,7 +44,8 @@ def sync_repositories(self):
             )
             for repo in repos:
                 remote_repository = self.create_repository(repo)
-                remote_repositories.append(remote_repository)
+                if remote_repository:
+                    remote_ids.append(remote_repository.remote_id)
 
         except (TypeError, ValueError):
             log.warning("Error syncing Bitbucket repositories")
@@ -74,12 +75,12 @@ def sync_repositories(self):
         except (TypeError, ValueError):
             pass
 
-        return remote_repositories
+        return remote_ids
 
     def sync_organizations(self):
         """Sync Bitbucket workspaces(our RemoteOrganization) and workspace repositories."""
-        remote_organizations = []
-        remote_repositories = []
+        organization_remote_ids = []
+        repository_remote_ids = []
 
         try:
             workspaces = self.paginate(
@@ -90,14 +91,15 @@ def sync_organizations(self):
                 remote_organization = self.create_organization(workspace)
                 repos = self.paginate(workspace["links"]["repositories"]["href"])
 
-                remote_organizations.append(remote_organization)
+                organization_remote_ids.append(remote_organization.remote_id)
 
                 for repo in repos:
                     remote_repository = self.create_repository(
                         repo,
                         organization=remote_organization,
                     )
-                    remote_repositories.append(remote_repository)
+                    if remote_repository:
+                        repository_remote_ids.append(remote_repository.remote_id)
 
         except ValueError:
             log.warning("Error syncing Bitbucket organizations")
@@ -107,7 +109,7 @@ def sync_organizations(self):
                 )
             )
 
-        return remote_organizations, remote_repositories
+        return organization_remote_ids, repository_remote_ids
 
     def create_repository(self, fields, privacy=None, organization=None):
         """

readthedocs/oauth/services/github.py

@@ -38,26 +38,27 @@ class GitHubService(UserService):
 
     def sync_repositories(self):
         """Sync repositories from GitHub API."""
-        remote_repositories = []
+        remote_ids = []
 
         try:
             repos = self.paginate(f"{self.base_api_url}/user/repos", per_page=100)
             for repo in repos:
                 remote_repository = self.create_repository(repo)
-                remote_repositories.append(remote_repository)
+                if remote_repository:
+                    remote_ids.append(remote_repository.remote_id)
         except (TypeError, ValueError):
             log.warning("Error syncing GitHub repositories")
             raise SyncServiceError(
                 SyncServiceError.INVALID_OR_REVOKED_ACCESS_TOKEN.format(
                     provider=self.vcs_provider_slug
                 )
             )
-        return remote_repositories
+        return remote_ids
 
     def sync_organizations(self):
         """Sync organizations from GitHub API."""
-        remote_organizations = []
-        remote_repositories = []
+        organization_remote_ids = []
+        repository_remote_ids = []
 
         try:
             orgs = self.paginate(f"{self.base_api_url}/user/orgs", per_page=100)
@@ -67,7 +68,7 @@ def sync_organizations(self):
                     org_details,
                     create_user_relationship=True,
                 )
-                remote_organizations.append(remote_organization)
+                organization_remote_ids.append(remote_organization.remote_id)
 
                 org_url = org["url"]
                 org_repos = self.paginate(
@@ -76,7 +77,8 @@ def sync_organizations(self):
                 )
                 for repo in org_repos:
                     remote_repository = self.create_repository(repo)
-                    remote_repositories.append(remote_repository)
+                    if remote_repository:
+                        repository_remote_ids.append(remote_repository.remote_id)
 
         except (TypeError, ValueError):
             log.warning("Error syncing GitHub organizations")
@@ -86,7 +88,7 @@ def sync_organizations(self):
                 )
             )
 
-        return remote_organizations, remote_repositories
+        return organization_remote_ids, repository_remote_ids
 
     def create_repository(self, fields, privacy=None):
         """
@@ -105,24 +107,11 @@ def create_repository(self, fields, privacy=None):
                 (fields["private"] is False and privacy == "public"),
             ]
         ):
-            repo, created = RemoteRepository.objects.get_or_create(
+            repo, _ = RemoteRepository.objects.get_or_create(
                 remote_id=str(fields["id"]),
                 vcs_provider=self.vcs_provider_slug,
             )
 
-            # TODO: For debugging: https://github.com/readthedocs/readthedocs.org/pull/9449.
-            if created:
-                _old_remote_repository = RemoteRepository.objects.filter(
-                    full_name=fields["full_name"], vcs_provider=self.vcs_provider_slug
-                ).first()
-                if _old_remote_repository:
-                    log.warning(
-                        "GitHub repository created with different remote_id but exact full_name.",
-                        fields=fields,
-                        old_remote_repository=_old_remote_repository.__dict__,
-                        imported=_old_remote_repository.projects.exists(),
-                    )
-
             owner_type = fields["owner"]["type"]
             organization = None
             if owner_type == "Organization":

readthedocs/oauth/services/gitlab.py

@@ -82,7 +82,7 @@ def sync_repositories(self):
 
         See https://docs.gitlab.com/api/projects/#list-a-users-projects.
         """
-        remote_repositories = []
+        remote_ids = []
         try:
             repos = self.paginate(
                 f"{self.base_api_url}/api/v4/users/{self.account.uid}/projects",
@@ -94,7 +94,8 @@ def sync_repositories(self):
 
             for repo in repos:
                 remote_repository = self.create_repository(repo)
-                remote_repositories.append(remote_repository)
+                if remote_repository:
+                    remote_ids.append(remote_repository.remote_id)
         except (TypeError, ValueError):
             log.warning("Error syncing GitLab repositories")
             raise SyncServiceError(
@@ -103,11 +104,11 @@ def sync_repositories(self):
                 )
             )
 
-        return remote_repositories
+        return remote_ids
 
     def sync_organizations(self):
-        remote_organizations = []
-        remote_repositories = []
+        organization_remote_ids = []
+        repository_remote_ids = []
 
         try:
             orgs = self.paginate(
@@ -130,7 +131,7 @@ def sync_organizations(self):
                     sort="asc",
                 )
 
-                remote_organizations.append(remote_organization)
+                organization_remote_ids.append(remote_organization.remote_id)
 
                 for repo in org_repos:
                     # TODO: Optimize this so that we don't re-fetch project data
@@ -151,7 +152,8 @@ def sync_organizations(self):
                             remote_repository = self.create_repository(
                                 repo_details, organization=remote_organization
                             )
-                            remote_repositories.append(remote_repository)
+                            if remote_repository:
+                                repository_remote_ids.append(remote_repository.remote_id)
                         else:
                             log.warning(
                                 "GitLab project does not exist or user does not have permissions.",
@@ -172,7 +174,7 @@ def sync_organizations(self):
                 )
             )
 
-        return remote_organizations, remote_repositories
+        return organization_remote_ids, repository_remote_ids
 
     def create_repository(
         self, fields, privacy=None, organization: RemoteOrganization | None = None

readthedocs/rtd_tests/tests/test_oauth_sync.py

@@ -194,13 +194,14 @@ def test_sync_repositories(self, mock_request):
         self.assertEqual(RemoteRepository.objects.count(), 0)
         self.assertEqual(RemoteOrganization.objects.count(), 0)
 
-        remote_repositories = self.service.sync_repositories()
+        repository_remote_ids = self.service.sync_repositories()
 
         self.assertEqual(RemoteRepository.objects.count(), 1)
         self.assertEqual(RemoteOrganization.objects.count(), 0)
-        self.assertEqual(len(remote_repositories), 1)
-        remote_repository = remote_repositories[0]
-        self.assertIsInstance(remote_repository, RemoteRepository)
+        self.assertEqual(len(repository_remote_ids), 1)
+
+        remote_repository = RemoteRepository.objects.first()
+        self.assertEqual(repository_remote_ids[0], remote_repository.remote_id)
         self.assertEqual(remote_repository.full_name, "organization/repository")
         self.assertEqual(remote_repository.name, "repository")
         self.assertFalse(remote_repository.remote_repository_relations.first().admin)
@@ -234,15 +235,15 @@ def test_sync_repositories_relation_with_organization(self, mock_request):
             organization=remote_organization,
             vcs_provider="github",
         )
-        remote_repositories = self.service.sync_repositories()
+        repository_remote_ids = self.service.sync_repositories()
 
         self.assertEqual(RemoteRepository.objects.count(), 1)
         self.assertEqual(RemoteRepositoryRelation.objects.count(), 1)
         self.assertEqual(RemoteOrganization.objects.count(), 1)
 
-        self.assertEqual(len(remote_repositories), 1)
-        remote_repository = remote_repositories[0]
-        self.assertIsInstance(remote_repository, RemoteRepository)
+        self.assertEqual(len(repository_remote_ids), 1)
+        remote_repository = RemoteRepository.objects.first()
+        self.assertEqual(repository_remote_ids[0], remote_repository.remote_id)
         self.assertEqual(remote_repository.full_name, "organization/repository")
         self.assertEqual(remote_repository.name, "repository")
         self.assertEqual(remote_repository.organization.slug, "organization")
@@ -276,15 +277,15 @@ def test_sync_repositories_moved_from_org_to_user(self, mock_request):
             organization=remote_organization,
             vcs_provider="github",
         )
-        remote_repositories = self.service.sync_repositories()
+        repository_remote_ids = self.service.sync_repositories()
 
         self.assertEqual(RemoteRepository.objects.count(), 1)
         self.assertEqual(RemoteRepositoryRelation.objects.count(), 1)
         self.assertEqual(RemoteOrganization.objects.count(), 1)
 
-        self.assertEqual(len(remote_repositories), 1)
-        remote_repository = remote_repositories[0]
-        self.assertIsInstance(remote_repository, RemoteRepository)
+        self.assertEqual(len(repository_remote_ids), 1)
+        remote_repository = RemoteRepository.objects.first()
+        self.assertEqual(repository_remote_ids[0], remote_repository.remote_id)
         self.assertEqual(remote_repository.full_name, "organization/repository")
         self.assertEqual(remote_repository.name, "repository")
         self.assertIsNone(remote_repository.organization)
@@ -407,14 +408,13 @@ def test_sync_organizations(self, mock_request):
         self.assertEqual(RemoteOrganization.objects.count(), 0)
         self.assertEqual(RemoteOrganizationRelation.objects.count(), 0)
 
-        remote_organizations, remote_repositories = self.service.sync_organizations()
+        organization_remote_ids, repository_remote_ids = self.service.sync_organizations()
 
         self.assertEqual(RemoteRepository.objects.count(), 1)
         self.assertEqual(RemoteRepositoryRelation.objects.count(), 1)
         self.assertEqual(RemoteOrganization.objects.count(), 1)
         self.assertEqual(RemoteOrganizationRelation.objects.count(), 1)
-        self.assertEqual(len(remote_organizations), 1)
-        self.assertEqual(len(remote_repositories), 1)
-        remote_organization = remote_organizations[0]
-        self.assertIsInstance(remote_organization, RemoteOrganization)
-        self.assertEqual(remote_organization.name, "Organization")
+        self.assertEqual(len(organization_remote_ids), 1)
+        self.assertEqual(len(repository_remote_ids), 1)
+        remote_organization = RemoteOrganization.objects.first()
+        self.assertEqual(organization_remote_ids[0], remote_organization.remote_id)