Refactor project documentation syncing tasks (#3143)

This updates the syncing tasks to execute in a guaranteed fashion. Before this
change, it was possible for broadcast tasks and symlink deletion tasks to
execute concurrently. This changes all the broadcast task calls into function
calls from a singular broadcast task.

In more detail:

* The `finish_build` task was executed as a task from the webs, and kicked off
  broadcast tasks to update files. Instead, Version update operations were move
  to the build process, through the API. The webs only the one broadcast task
* Version API was given the same APIRestrictedPermission as other end points, it
  can now be updated by staff

Author: agjohnson

readthedocs/projects/tasks.py

@@ -214,17 +214,14 @@ def run_build(self, docker=False, record=True):
             except SoftTimeLimitExceeded:
                 raise BuildEnvironmentError(_('Build exited due to time out'))
 
-            # Web Server Tasks
+            # Finalize build and update web servers
             if build_id:
-                finish_build.delay(
-                    version_pk=self.version.pk,
-                    build_pk=build_id,
-                    hostname=socket.gethostname(),
-                    html=outcomes['html'],
-                    search=outcomes['search'],
-                    localmedia=outcomes['localmedia'],
-                    pdf=outcomes['pdf'],
-                    epub=outcomes['epub'],
+                self.update_app_instances(
+                    html=bool(outcomes['html']),
+                    search=bool(outcomes['search']),
+                    localmedia=bool(outcomes['localmedia']),
+                    pdf=bool(outcomes['pdf']),
+                    epub=bool(outcomes['epub']),
                 )
 
         if self.build_env.failed:
@@ -333,6 +330,51 @@ def update_documentation_type(self):
         api_v2.project(self.project.pk).put(project_data)
         self.project.documentation_type = ret
 
+    def update_app_instances(self, html=False, localmedia=False, search=False,
+                             pdf=False, epub=False):
+        """Update application instances with build artifacts
+
+        This triggers updates across application instances for html, pdf, epub,
+        downloads, and search. Tasks are broadcast to all web servers from here.
+        """
+        # Update version if we have successfully built HTML output
+        try:
+            if html:
+                version = api_v2.version(self.version.pk)
+                version.patch({
+                    'active': True,
+                    'built': True,
+                })
+        except HttpClientError as e:
+            log.error('Updating version failed, skipping file sync: version=%s',
+                      self.version.pk, exc_info=True)
+        else:
+            # Broadcast finalization steps to web application instances
+            broadcast(
+                type='app',
+                task=sync_files,
+                args=[
+                    self.project.pk,
+                    self.version.pk,
+                ],
+                kwargs=dict(
+                    hostname=socket.gethostname(),
+                    html=html,
+                    localmedia=localmedia,
+                    search=search,
+                    pdf=pdf,
+                    epub=epub,
+                )
+            )
+
+            # Delayed tasks
+            # TODO these should be chained on to the broadcast calls. The
+            # broadcast calls could be lumped together into a promise, and on
+            # task result, these next few tasks can be updated, also in a
+            # chained fashion
+            fileify.delay(self.version.pk, commit=self.build.get('commit'))
+            update_search.delay(self.version.pk, commit=self.build.get('commit'))
+
     def setup_environment(self):
         """
         Build the virtualenv and install the project into it.
@@ -541,41 +583,35 @@ def update_imported_docs(version_pk):
 
 # Web tasks
 @task(queue='web')
-def finish_build(version_pk, build_pk, hostname=None, html=False,
-                 localmedia=False, search=False, pdf=False, epub=False):
-    """Build Finished, do house keeping bits"""
-    version = Version.objects.get(pk=version_pk)
-    build = Build.objects.get(pk=build_pk)
-
-    if html:
-        version.active = True
-        version.built = True
-        version.save()
+def sync_files(project_pk, version_pk, hostname=None, html=False,
+               localmedia=False, search=False, pdf=False, epub=False):
+    """Sync build artifacts to application instances
 
+    This task broadcasts from a build instance on build completion and
+    performs synchronization of build artifacts on each application instance.
+    """
+    # Clean up unused artifacts
     if not pdf:
-        broadcast(type='app', task=clear_pdf_artifacts, args=[version.pk])
+        clear_pdf_artifacts(version_pk)
     if not epub:
-        broadcast(type='app', task=clear_epub_artifacts, args=[version.pk])
+        clear_epub_artifacts(version_pk)
 
     # Sync files to the web servers
-    broadcast(type='app', task=move_files, args=[version_pk, hostname],
-              kwargs=dict(
-                  html=html,
-                  localmedia=localmedia,
-                  search=search,
-                  pdf=pdf,
-                  epub=epub,
-    ))
+    move_files(
+        version_pk,
+        hostname,
+        html=html,
+        localmedia=localmedia,
+        search=search,
+        pdf=pdf,
+        epub=epub
+    )
 
-    # Symlink project on every web
-    broadcast(type='app', task=symlink_project, args=[version.project.pk])
+    # Symlink project
+    symlink_project(project_pk)
 
     # Update metadata
-    broadcast(type='app', task=update_static_metadata, args=[version.project.pk])
-
-    # Delayed tasks
-    fileify.delay(version.pk, commit=build.commit)
-    update_search.delay(version.pk, commit=build.commit)
+    update_static_metadata(project_pk)
 
 
 @task(queue='web')

readthedocs/restapi/views/model_views.py

@@ -156,8 +156,9 @@ def sync_versions(self, request, **kwargs):
         })
 
 
-class VersionViewSet(viewsets.ReadOnlyModelViewSet):
-    permission_classes = [permissions.IsAuthenticatedOrReadOnly]
+class VersionViewSet(viewsets.ModelViewSet):
+
+    permission_classes = [APIRestrictedPermission]
     renderer_classes = (JSONRenderer,)
     serializer_class = VersionSerializer
     model = Version