Abstraction for UpdateDocsTask and SyncRepositoryTask

humitos · humitos · commit c79f9f12fa97 · 2018-01-23T10:54:49.000-05:00
These are two separated tasks that share some code by inheriting from
SyncRepositoryMixin.

The final goal (as ``core.views.hooks._build_url`` is *DEPRECATED*) is
to finally remove the SyncRepositoryTask and merge that code into
UpdateDocsTask.
diff --git a/readthedocs/core/management/commands/pull.py b/readthedocs/core/management/commands/pull.py
@@ -20,8 +20,6 @@ def handle(self, *args, **options):
         if args:
             for slug in args:
                 version = utils.version_from_slug(slug, LATEST)
-                tasks.UpdateDocsTask().run(
-                    version.project,
+                tasks.SyncRepositoryTask().run(
                     version.pk,
-                    sync_only=True,
                 )
diff --git a/readthedocs/core/views/hooks.py b/readthedocs/core/views/hooks.py
@@ -12,7 +12,7 @@
 from readthedocs.builds.constants import LATEST
 from readthedocs.projects import constants
 from readthedocs.projects.models import Project, Feature
-from readthedocs.projects.tasks import UpdateDocsTask
+from readthedocs.projects.tasks import SyncRepositoryTask
 
 import logging
 
@@ -123,15 +123,11 @@ def _build_url(url, projects, branches):
     for project in projects:
         (built, not_building) = build_branches(project, branches)
         if not built:
-            # Call UpdateDocsTask with ``sync_only`` to update tag/branch info
+            # Call SyncRepositoryTask to update tag/branch info
             version = project.versions.get(slug=LATEST)
-            update_docs = UpdateDocsTask()
-            update_docs.apply_async(
-                args=(project.pk,),
-                kwargs={
-                    'version_pk': version.pk,
-                    'sync_only': True,
-                },
+            sync_repository = SyncRepositoryTask()
+            sync_repository.apply_async(
+                args=(version.pk,),
             )
             msg = '(URL Build) Syncing versions for %s' % project.slug
             log.info(msg)
diff --git a/readthedocs/projects/apps.py b/readthedocs/projects/apps.py
@@ -7,6 +7,7 @@ class ProjectsConfig(AppConfig):
     name = 'readthedocs.projects'
 
     def ready(self):
-        from readthedocs.projects.tasks import UpdateDocsTask
+        from readthedocs.projects import tasks
         from readthedocs.worker import app
-        app.tasks.register(UpdateDocsTask)
+        app.tasks.register(tasks.SyncRepositoryTask)
+        app.tasks.register(tasks.UpdateDocsTask)
diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py
@@ -63,7 +63,132 @@
 HTML_ONLY = getattr(settings, 'HTML_ONLY_PROJECTS', ())
 
 
-class UpdateDocsTask(Task):
+class SyncRepositoryMixin(object):
+
+    """
+    Mixin that handles the VCS sync/update.
+    """
+
+    @staticmethod
+    def get_version(project=None, version_pk=None):
+        """
+        Retrieve version data from the API.
+
+        :param project: project object to sync
+        :type project: projects.models.Project
+        :param version_pk: version pk to sync
+        :type version_pk: int
+        :returns: a data-complete version object
+        :rtype: builds.models.APIVersion
+        """
+        assert (project or version_pk), 'project or version_pk is needed'
+        if version_pk:
+            version_data = api_v2.version(version_pk).get()
+        else:
+            version_data = (api_v2
+                            .version(project.slug)
+                            .get(slug=LATEST)['objects'][0])
+        return APIVersion(**version_data)
+
+    def sync_repo(self):
+        """
+        Checkout/update the project's repository and hit ``sync_versions`` API.
+        """
+        # Make Dirs
+        if not os.path.exists(self.project.doc_path):
+            os.makedirs(self.project.doc_path)
+
+        if not self.project.vcs_repo():
+            raise RepositoryError(
+                _('Repository type "{repo_type}" unknown').format(
+                    repo_type=self.project.repo_type,
+                ),
+            )
+
+        with self.project.repo_nonblockinglock(
+                version=self.version,
+                max_lock_age=getattr(settings, 'REPO_LOCK_SECONDS', 30)):
+
+            # Get the actual code on disk
+            try:
+                before_vcs.send(sender=self.version)
+                self._log(
+                    'Checking out version {slug}: {identifier}'.format(
+                        slug=self.version.slug,
+                        identifier=self.version.identifier,
+                    ),
+                )
+                version_repo = self.project.vcs_repo(
+                    self.version.slug,
+                    # When called from ``SyncRepositoryTask.run`` we don't have
+                    # a ``setup_env`` so we use just ``None`` and commands won't
+                    # be recorded
+                    getattr(self, 'setup_env', None),
+                )
+                version_repo.checkout(self.version.identifier)
+            finally:
+                after_vcs.send(sender=self.version)
+
+            # Update tags/version
+            version_post_data = {'repo': version_repo.repo_url}
+
+            if version_repo.supports_tags:
+                version_post_data['tags'] = [
+                    {'identifier': v.identifier,
+                     'verbose_name': v.verbose_name,
+                     } for v in version_repo.tags
+                ]
+
+            if version_repo.supports_branches:
+                version_post_data['branches'] = [
+                    {'identifier': v.identifier,
+                     'verbose_name': v.verbose_name,
+                     } for v in version_repo.branches
+                ]
+
+            try:
+                # Hit the API ``sync_versions`` which may trigger a new build
+                # for the stable version
+                api_v2.project(self.project.pk).sync_versions.post(version_post_data)
+            except HttpClientError:
+                log.exception('Sync Versions Exception')
+            except Exception:
+                log.exception('Unknown Sync Versions Exception')
+
+
+class SyncRepositoryTask(SyncRepositoryMixin, Task):
+
+    """
+    Entry point to synchronize the VCS documentation.
+    """
+
+    max_retries = 5
+    default_retry_delay = (7 * 60)
+    name = __name__ + '.sync_repository'
+
+    def run(self, version_pk):
+        """
+        Run the VCS synchronization.
+
+        :param version_pk: version pk to sync
+        :type version_pk: int
+        :returns: whether or not the task ended successfully
+        :rtype: bool
+        """
+        try:
+            self.version = self.get_version(version_pk=version_pk)
+            self.project = self.version.project
+            self.sync_repo()
+            return True
+        # Catch unhandled errors when syncing
+        except Exception:
+            log.exception(
+                'An unhandled exception was raised during VCS syncing',
+            )
+            return False
+
+
+class UpdateDocsTask(SyncRepositoryMixin, Task):
 
     """
     The main entry point for updating documentation.
@@ -106,8 +231,7 @@ def _log(self, msg):
 
     # pylint: disable=arguments-differ
     def run(self, pk, version_pk=None, build_pk=None, record=True,
-            docker=False, search=True, force=False, localmedia=True,
-            sync_only=False, **__):
+            docker=False, search=True, force=False, localmedia=True, **__):
         """
         Run a documentation sync or sync n' build.
 
@@ -145,10 +269,6 @@ def run(self, pk, version_pk=None, build_pk=None, record=True,
             self.build_force = force
             self.config = None
 
-            if sync_only:
-                self.sync_repo()
-                return True
-
             setup_successful = self.run_setup(record=record)
             if not setup_successful:
                 return False
@@ -301,17 +421,6 @@ def get_project(project_pk):
         project_data = api_v2.project(project_pk).get()
         return APIProject(**project_data)
 
-    @staticmethod
-    def get_version(project, version_pk):
-        """Ensure we're using a sane version."""
-        if version_pk:
-            version_data = api_v2.version(version_pk).get()
-        else:
-            version_data = (api_v2
-                            .version(project.slug)
-                            .get(slug=LATEST)['objects'][0])
-        return APIVersion(**version_data)
-
     @staticmethod
     def get_build(build_pk):
         """
@@ -342,70 +451,6 @@ def setup_vcs(self):
         if commit:
             self.build['commit'] = commit
 
-    def sync_repo(self):
-        """
-        Checkout/update the project's repository and hit ``sync_versions`` API.
-        """
-        # Make Dirs
-        if not os.path.exists(self.project.doc_path):
-            os.makedirs(self.project.doc_path)
-
-        if not self.project.vcs_repo():
-            raise RepositoryError(
-                _('Repository type "{repo_type}" unknown').format(
-                    repo_type=self.project.repo_type,
-                ),
-            )
-
-        with self.project.repo_nonblockinglock(
-                version=self.version,
-                max_lock_age=getattr(settings, 'REPO_LOCK_SECONDS', 30)):
-
-            # Get the actual code on disk
-            try:
-                before_vcs.send(sender=self.version)
-                self._log(
-                    'Checking out version {slug}: {identifier}'.format(
-                        slug=self.version.slug,
-                        identifier=self.version.identifier,
-                    ),
-                )
-                version_repo = self.project.vcs_repo(
-                    self.version.slug,
-                    # When ``sync_only`` we don't a setup_env
-                    getattr(self, 'setup_env', None),
-                )
-                version_repo.checkout(self.version.identifier)
-            finally:
-                after_vcs.send(sender=self.version)
-
-            # Update tags/version
-            version_post_data = {'repo': version_repo.repo_url}
-
-            if version_repo.supports_tags:
-                version_post_data['tags'] = [
-                    {'identifier': v.identifier,
-                     'verbose_name': v.verbose_name,
-                     } for v in version_repo.tags
-                ]
-
-            if version_repo.supports_branches:
-                version_post_data['branches'] = [
-                    {'identifier': v.identifier,
-                     'verbose_name': v.verbose_name,
-                     } for v in version_repo.branches
-                ]
-
-            try:
-                # Hit the API ``sync_versions`` which may trigger a new build
-                # for the stable version
-                api_v2.project(self.project.pk).sync_versions.post(version_post_data)
-            except HttpClientError:
-                log.exception('Sync Versions Exception')
-            except Exception:
-                log.exception('Unknown Sync Versions Exception')
-
-
     def get_env_vars(self):
         """Get bash environment variables used for all builder commands."""
         env = {
diff --git a/readthedocs/rtd_tests/tests/test_celery.py b/readthedocs/rtd_tests/tests/test_celery.py
@@ -8,7 +8,7 @@
 from django_dynamic_fixture import get
 from mock import patch, MagicMock
 
-from readthedocs.builds.constants import BUILD_STATE_INSTALLING, BUILD_STATE_FINISHED
+from readthedocs.builds.constants import BUILD_STATE_INSTALLING, BUILD_STATE_FINISHED, LATEST
 from readthedocs.builds.models import Build
 from readthedocs.projects.models import Project
 from readthedocs.projects import tasks
@@ -115,11 +115,11 @@ def test_update_docs_unexpected_build_exception(self, mock_build_docs):
                 intersphinx=False)
         self.assertTrue(result.successful())
 
-    def test_update_imported_doc(self):
+    def test_sync_repository(self):
+        version = self.project.versions.get(slug=LATEST)
         with mock_api(self.repo):
-            update_docs = tasks.UpdateDocsTask()
-            result = update_docs.apply_async(
-                args=(self.project.pk,),
-                kwargs={'sync_only': True},
+            sync_repository = tasks.SyncRepositoryTask()
+            result = sync_repository.apply_async(
+                args=(version.pk,),
             )
         self.assertTrue(result.successful())
