Migrate sync_versions from an API call to a task (#7548)

This should stop the API from timing out,
since we aren’t running all this logic in process.
This is most important for projects with lots of versions,
because this logic takes a long time to run and times out the web processes.
It also eats up our API processes,
causing other issues with our API because of blocking.

To test this, you can use the `pull` management command on a project you have checked out locally:

```
inv -e docker.manage 'pull time-test’
```

Co-authored-by: Santos Gallegos <[email protected]>
Co-authored-by: Manuel Kaufmann <[email protected]>

Author: 3 people

.gitignore

@@ -39,6 +39,7 @@ media/pdf
 media/static
 /static
 node_modules
+nodeenv
 readthedocs/rtd_tests/builds
 readthedocs/rtd_tests/tests/builds
 user_builds

readthedocs/api/v2/utils.py

@@ -166,14 +166,16 @@ def _set_or_create_version(project, slug, version_id, verbose_name, type_):
     return version, False
 
 
-def _get_deleted_versions_qs(project, version_data):
+def _get_deleted_versions_qs(project, tags_data, branches_data):
     # We use verbose_name for tags
     # because several tags can point to the same identifier.
     versions_tags = [
-        version['verbose_name'] for version in version_data.get('tags', [])
+        version['verbose_name']
+        for version in tags_data
     ]
     versions_branches = [
-        version['identifier'] for version in version_data.get('branches', [])
+        version['identifier']
+        for version in branches_data
     ]
 
     to_delete_qs = (
@@ -193,14 +195,18 @@ def _get_deleted_versions_qs(project, version_data):
     return to_delete_qs
 
 
-def delete_versions_from_db(project, version_data):
+def delete_versions_from_db(project, tags_data, branches_data):
     """
     Delete all versions not in the current repo.
 
     :returns: The slug of the deleted versions from the database.
     """
     to_delete_qs = (
-        _get_deleted_versions_qs(project, version_data)
+        _get_deleted_versions_qs(
+            project=project,
+            tags_data=tags_data,
+            branches_data=branches_data,
+        )
         .exclude(active=True)
     )
     deleted_versions = set(to_delete_qs.values_list('slug', flat=True))
@@ -214,10 +220,14 @@ def delete_versions_from_db(project, version_data):
     return deleted_versions
 
 
-def get_deleted_active_versions(project, version_data):
+def get_deleted_active_versions(project, tags_data, branches_data):
     """Return the slug of active versions that were deleted from the repository."""
     to_delete_qs = (
-        _get_deleted_versions_qs(project, version_data)
+        _get_deleted_versions_qs(
+            project=project,
+            tags_data=tags_data,
+            branches_data=branches_data,
+        )
         .filter(active=True)
     )
     return set(to_delete_qs.values_list('slug', flat=True))

readthedocs/api/v2/views/model_views.py

@@ -6,35 +6,21 @@
 from allauth.socialaccount.models import SocialAccount
 from django.conf import settings
 from django.core.files.storage import get_storage_class
-from django.db.models import Q
 from django.shortcuts import get_object_or_404
 from django.template.loader import render_to_string
 from rest_framework import decorators, permissions, status, viewsets
 from rest_framework.parsers import JSONParser, MultiPartParser
 from rest_framework.renderers import BaseRenderer, JSONRenderer
 from rest_framework.response import Response
 
-from readthedocs.builds.constants import (
-    BRANCH,
-    BUILD_STATE_FINISHED,
-    BUILD_STATE_TRIGGERED,
-    INTERNAL,
-    TAG,
-)
+from readthedocs.builds.constants import INTERNAL
 from readthedocs.builds.models import Build, BuildCommandResult, Version
-from readthedocs.core.utils import trigger_build
-from readthedocs.core.utils.extend import SettingsOverrideObject
+from readthedocs.builds.tasks import sync_versions_task
 from readthedocs.oauth.models import RemoteOrganization, RemoteRepository
 from readthedocs.oauth.services import GitHubService, registry
-from readthedocs.projects.models import Domain, EmailHook, Project
-from readthedocs.projects.version_handling import determine_stable_version
-
-from ..permissions import (
-    APIPermission,
-    APIRestrictedPermission,
-    IsOwner,
-    RelatedProjectIsOwner,
-)
+from readthedocs.projects.models import Domain, Project
+
+from ..permissions import APIPermission, APIRestrictedPermission, IsOwner
 from ..serializers import (
     BuildAdminSerializer,
     BuildCommandSerializer,
@@ -52,10 +38,6 @@
     ProjectPagination,
     RemoteOrganizationPagination,
     RemoteProjectPagination,
-    delete_versions_from_db,
-    get_deleted_active_versions,
-    run_automation_rules,
-    sync_versions_to_db,
 )
 
 log = logging.getLogger(__name__)
@@ -181,47 +163,34 @@ def canonical_url(self, request, **kwargs):
         permission_classes=[permissions.IsAdminUser],
         methods=['post'],
     )
-    def sync_versions(self, request, **kwargs):  # noqa: D205
+    def sync_versions(self, request, **kwargs):  # noqa
         """
         Sync the version data in the repo (on the build server).
 
         Version data in the repo is synced with what we have in the database.
 
         :returns: the identifiers for the versions that have been deleted.
+
+        .. note::
+
+           This endpoint is deprecated in favor of `sync_versions_task`.
         """
         project = get_object_or_404(
             Project.objects.api(request.user),
             pk=kwargs['pk'],
         )
 
-        # If the currently highest non-prerelease version is active, then make
-        # the new latest version active as well.
-        current_stable = project.get_original_stable_version()
-        if current_stable is not None:
-            activate_new_stable = current_stable.active
-        else:
-            activate_new_stable = False
+        added_versions = []
+        deleted_versions = []
 
         try:
-            # Update All Versions
             data = request.data
-            added_versions = set()
-            if 'tags' in data:
-                ret_set = sync_versions_to_db(
-                    project=project,
-                    versions=data['tags'],
-                    type=TAG,
-                )
-                added_versions.update(ret_set)
-            if 'branches' in data:
-                ret_set = sync_versions_to_db(
-                    project=project,
-                    versions=data['branches'],
-                    type=BRANCH,
-                )
-                added_versions.update(ret_set)
-            deleted_versions = delete_versions_from_db(project, data)
-            deleted_active_versions = get_deleted_active_versions(project, data)
+            # Calling the task synchronically to keep backward compatibility
+            added_versions, deleted_versions = sync_versions_task(
+                project_pk=project.pk,
+                tags_data=data.get('tags', []),
+                branches_data=data.get('branches', []),
+            )
         except Exception as e:
             log.exception('Sync Versions Error')
             return Response(
@@ -231,42 +200,6 @@ def sync_versions(self, request, **kwargs):  # noqa: D205
                 status=status.HTTP_400_BAD_REQUEST,
             )
 
-        try:
-            # The order of added_versions isn't deterministic.
-            # We don't track the commit time or any other metadata.
-            # We usually have one version added per webhook.
-            run_automation_rules(project, added_versions, deleted_active_versions)
-        except Exception:
-            # Don't interrupt the request if something goes wrong
-            # in the automation rules.
-            log.exception(
-                'Failed to execute automation rules for [%s]: %s',
-                project.slug, added_versions
-            )
-
-        # TODO: move this to an automation rule
-        promoted_version = project.update_stable_version()
-        new_stable = project.get_stable_version()
-        if promoted_version and new_stable and new_stable.active:
-            log.info(
-                'Triggering new stable build: %(project)s:%(version)s',
-                {
-                    'project': project.slug,
-                    'version': new_stable.identifier,
-                }
-            )
-            trigger_build(project=project, version=new_stable)
-
-            # Marking the tag that is considered the new stable version as
-            # active and building it if it was just added.
-            if (
-                activate_new_stable and
-                promoted_version.slug in added_versions
-            ):
-                promoted_version.active = True
-                promoted_version.save()
-                trigger_build(project=project, version=promoted_version)
-
         return Response({
             'added_versions': added_versions,
             'deleted_versions': deleted_versions,

readthedocs/builds/tasks.py

@@ -8,14 +8,24 @@
 from django.core.files.storage import get_storage_class
 
 from readthedocs.api.v2.serializers import BuildSerializer
+from readthedocs.api.v2.utils import (
+    delete_versions_from_db,
+    get_deleted_active_versions,
+    run_automation_rules,
+    sync_versions_to_db,
+)
 from readthedocs.builds.constants import (
+    BRANCH,
     BUILD_STATUS_FAILURE,
     BUILD_STATUS_PENDING,
     BUILD_STATUS_SUCCESS,
     MAX_BUILD_COMMAND_SIZE,
+    TAG,
 )
 from readthedocs.builds.models import Build, Version
 from readthedocs.builds.utils import memcache_lock
+from readthedocs.core.utils import trigger_build
+from readthedocs.projects.models import Project
 from readthedocs.projects.tasks import send_build_status
 from readthedocs.worker import app
 
@@ -206,3 +216,99 @@ def delete_inactive_external_versions(limit=200, days=30 * 3):
                 version.project.slug, version.slug,
             )
             version.delete()
+
+
+@app.task(
+    max_retries=1,
+    default_retry_delay=60,
+    queue='web'
+)
+def sync_versions_task(project_pk, tags_data, branches_data, **kwargs):
+    """
+    Sync the version data in the repo (from build server) into our database.
+
+    Creates new Version objects for tags/branches that aren't tracked in the database,
+    and deletes Version objects for tags/branches that don't exists in the repository.
+
+    :param tags_data: List of dictionaries with ``verbose_name`` and ``identifier``.
+    :param branches_data: Same as ``tags_data`` but for branches.
+    :returns: the identifiers for the versions that have been deleted.
+    """
+    project = Project.objects.get(pk=project_pk)
+
+    # If the currently highest non-prerelease version is active, then make
+    # the new latest version active as well.
+    current_stable = project.get_original_stable_version()
+    if current_stable is not None:
+        activate_new_stable = current_stable.active
+    else:
+        activate_new_stable = False
+
+    try:
+        # Update All Versions
+        added_versions = set()
+        result = sync_versions_to_db(
+            project=project,
+            versions=tags_data,
+            type=TAG,
+        )
+        added_versions.update(result)
+
+        result = sync_versions_to_db(
+            project=project,
+            versions=branches_data,
+            type=BRANCH,
+        )
+        added_versions.update(result)
+
+        deleted_versions = delete_versions_from_db(
+            project=project,
+            tags_data=tags_data,
+            branches_data=branches_data,
+        )
+        deleted_active_versions = get_deleted_active_versions(
+            project=project,
+            tags_data=tags_data,
+            branches_data=branches_data,
+        )
+    except Exception:
+        log.exception('Sync Versions Error')
+        return [], []
+
+    try:
+        # The order of added_versions isn't deterministic.
+        # We don't track the commit time or any other metadata.
+        # We usually have one version added per webhook.
+        run_automation_rules(project, added_versions, deleted_active_versions)
+    except Exception:
+        # Don't interrupt the request if something goes wrong
+        # in the automation rules.
+        log.exception(
+            'Failed to execute automation rules for [%s]: %s',
+            project.slug, added_versions
+        )
+
+    # TODO: move this to an automation rule
+    promoted_version = project.update_stable_version()
+    new_stable = project.get_stable_version()
+    if promoted_version and new_stable and new_stable.active:
+        log.info(
+            'Triggering new stable build: %(project)s:%(version)s',
+            {
+                'project': project.slug,
+                'version': new_stable.identifier,
+            }
+        )
+        trigger_build(project=project, version=new_stable)
+
+        # Marking the tag that is considered the new stable version as
+        # active and building it if it was just added.
+        if (
+            activate_new_stable and
+            promoted_version.slug in added_versions
+        ):
+            promoted_version.active = True
+            promoted_version.save()
+            trigger_build(project=project, version=promoted_version)
+
+    return list(added_versions), list(deleted_versions)

readthedocs/core/management/commands/pull.py

@@ -4,7 +4,7 @@
 
 import logging
 
-from django.core.management.base import BaseCommand
+from django.core.management.base import LabelCommand
 
 from readthedocs.builds.constants import LATEST
 from readthedocs.projects import tasks, utils
@@ -13,12 +13,10 @@
 log = logging.getLogger(__name__)
 
 
-class Command(BaseCommand):
+class Command(LabelCommand):
 
     help = __doc__
 
-    def handle(self, *args, **options):
-        if args:
-            for slug in args:
-                version = utils.version_from_slug(slug, LATEST)
-                tasks.sync_repository_task(version.pk)
+    def handle_label(self, label, **options):
+        version = utils.version_from_slug(label, LATEST)
+        tasks.sync_repository_task(version.pk)