From 257a96da05dc3d4e76b158dd60c775a11dd3de1b Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Wed, 4 Oct 2017 13:48:04 -0600 Subject: [PATCH] Refactor project documentation syncing tasks 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 --- readthedocs/projects/tasks.py | 110 +++++++++++++++-------- readthedocs/restapi/views/model_views.py | 5 +- 2 files changed, 76 insertions(+), 39 deletions(-) diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 40bbfb99a75..6db7498b70e 100644 --- a/readthedocs/projects/tasks.py +++ b/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') diff --git a/readthedocs/restapi/views/model_views.py b/readthedocs/restapi/views/model_views.py index 24f15713a8f..b5836fdb4cb 100644 --- a/readthedocs/restapi/views/model_views.py +++ b/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