diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index e67538be43c..8c4bac3ea44 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -6,6 +6,7 @@ from __future__ import absolute_import +import datetime import hashlib import json import logging @@ -20,6 +21,7 @@ from celery.exceptions import SoftTimeLimitExceeded from django.conf import settings from django.core.urlresolvers import reverse +from django.db.models import Q from django.utils.translation import ugettext_lazy as _ from readthedocs_build.config import ConfigError from slumber.exceptions import HttpClientError @@ -41,6 +43,7 @@ from readthedocs.core.symlink import PublicSymlink, PrivateSymlink from readthedocs.core.utils import send_email, broadcast from readthedocs.doc_builder.config import load_yaml_config +from readthedocs.doc_builder.constants import DOCKER_LIMITS from readthedocs.doc_builder.environments import (LocalEnvironment, DockerEnvironment) from readthedocs.doc_builder.exceptions import BuildEnvironmentError @@ -1005,3 +1008,48 @@ def sync_callback(_, version_pk, commit, *args, **kwargs): """ fileify(version_pk, commit=commit) update_search(version_pk, commit=commit) + + +@app.task() +def finish_inactive_builds(): + """ + Finish inactive builds. + + A build is consider inactive if it's not in ``FINISHED`` state and it has been + "running" for more time that the allowed one (``Project.container_time_limit`` + or ``DOCKER_LIMITS['time']`` plus a 20% of it). + + These inactive builds will be marked as ``success`` and ``FINISHED`` with an + ``error`` to be communicated to the user. + """ + time_limit = int(DOCKER_LIMITS['time'] * 1.2) + delta = datetime.timedelta(seconds=time_limit) + query = (~Q(state=BUILD_STATE_FINISHED) & + Q(date__lte=datetime.datetime.now() - delta)) + + builds_finished = 0 + builds = Build.objects.filter(query)[:50] + for build in builds: + + if build.project.container_time_limit: + custom_delta = datetime.timedelta( + seconds=int(build.project.container_time_limit)) + if build.date + custom_delta > datetime.datetime.now(): + # Do not mark as FINISHED builds with a custom time limit that wasn't + # expired yet (they are still building the project version) + continue + + build.success = False + build.state = BUILD_STATE_FINISHED + build.error = _( + 'This build was terminated due to inactivity. If you ' + 'continue to encounter this error, file a support ' + 'request with and reference this build id ({0}).'.format(build.pk) + ) + build.save() + builds_finished += 1 + + log.info( + 'Builds marked as "Terminated due inactivity": %s', + builds_finished, + ) diff --git a/readthedocs/restapi/views/footer_views.py b/readthedocs/restapi/views/footer_views.py index 295d0486155..c4e3b451732 100644 --- a/readthedocs/restapi/views/footer_views.py +++ b/readthedocs/restapi/views/footer_views.py @@ -43,7 +43,7 @@ def get_version_compare_data(project, base_version=None): } if highest_version_obj: ret_val['url'] = highest_version_obj.get_absolute_url() - ret_val['slug'] = highest_version_obj.slug, + ret_val['slug'] = (highest_version_obj.slug,) if base_version and base_version.slug != LATEST: try: base_version_comparable = parse_version_failsafe( diff --git a/readthedocs/rtd_tests/tests/test_project.py b/readthedocs/rtd_tests/tests/test_project.py index 672dacce3f2..88eb78ee20f 100644 --- a/readthedocs/rtd_tests/tests/test_project.py +++ b/readthedocs/rtd_tests/tests/test_project.py @@ -1,16 +1,24 @@ -from __future__ import absolute_import +# -*- coding: utf-8 -*- +from __future__ import ( + absolute_import, division, print_function, unicode_literals) + +import datetime import json + from django.test import TestCase -from readthedocs.builds.constants import LATEST -from readthedocs.projects.models import Project -from rest_framework.reverse import reverse from django_dynamic_fixture import get -from readthedocs.restapi.serializers import ProjectSerializer +from rest_framework.reverse import reverse + +from readthedocs.builds.constants import ( + BUILD_STATE_CLONING, BUILD_STATE_FINISHED, BUILD_STATE_TRIGGERED, LATEST) +from readthedocs.builds.models import Build +from readthedocs.projects.models import Project +from readthedocs.projects.tasks import finish_inactive_builds from readthedocs.rtd_tests.mocks.paths import fake_paths_by_regex class TestProject(TestCase): - fixtures = ["eric", "test_data"] + fixtures = ['eric', 'test_data'] def setUp(self): self.client.login(username='eric', password='test') @@ -40,17 +48,19 @@ def test_translations(self): self.assertEqual(response.status_code, 200) translation_ids_from_api = [ - t['id'] for t in response.data['translations']] + t['id'] for t in response.data['translations'] + ] translation_ids_from_orm = [ - t[0] for t in main_project.translations.values_list('id')] + t[0] for t in main_project.translations.values_list('id') + ] self.assertEqual( set(translation_ids_from_api), - set(translation_ids_from_orm) + set(translation_ids_from_orm), ) def test_translation_delete(self): - """Ensure translation deletion doesn't cascade up to main project""" + """Ensure translation deletion doesn't cascade up to main project.""" # In this scenario, a user has created a project and set the translation # to another project. If the user deletes this new project, the delete # operation shouldn't cascade up to the main project, and should instead @@ -62,12 +72,13 @@ def test_translation_delete(self): self.assertTrue(Project.objects.filter(pk=project_delete.pk).exists()) self.assertEqual( Project.objects.get(pk=project_keep.pk).main_language_project, - project_delete + project_delete, ) project_delete.delete() self.assertFalse(Project.objects.filter(pk=project_delete.pk).exists()) self.assertTrue(Project.objects.filter(pk=project_keep.pk).exists()) - self.assertIsNone(Project.objects.get(pk=project_keep.pk).main_language_project) + self.assertIsNone( + Project.objects.get(pk=project_keep.pk).main_language_project) def test_token(self): r = self.client.get('/api/v2/project/6/token/', {}) @@ -104,3 +115,63 @@ def test_has_epub_with_epub_build_disabled(self): self.pip.enable_epub_build = False with fake_paths_by_regex('\.epub$'): self.assertFalse(self.pip.has_epub(LATEST)) + + +class TestFinishInactiveBuildsTask(TestCase): + fixtures = ['eric', 'test_data'] + + def setUp(self): + self.client.login(username='eric', password='test') + self.pip = Project.objects.get(slug='pip') + + self.taggit = Project.objects.get(slug='taggit') + self.taggit.container_time_limit = 7200 # 2 hours + self.taggit.save() + + # Build just started with the default time + self.build_1 = Build.objects.create( + project=self.pip, + version=self.pip.get_stable_version(), + state=BUILD_STATE_CLONING, + ) + + # Build started an hour ago with default time + self.build_2 = Build.objects.create( + project=self.pip, + version=self.pip.get_stable_version(), + state=BUILD_STATE_TRIGGERED, + ) + self.build_2.date = ( + datetime.datetime.now() - datetime.timedelta(hours=1)) + self.build_2.save() + + # Build started an hour ago with custom time (2 hours) + self.build_3 = Build.objects.create( + project=self.taggit, + version=self.taggit.get_stable_version(), + state=BUILD_STATE_TRIGGERED, + ) + self.build_3.date = ( + datetime.datetime.now() - datetime.timedelta(hours=1)) + self.build_3.save() + + def test_finish_inactive_builds_task(self): + finish_inactive_builds() + + # Legitimate build (just started) not finished + self.build_1.refresh_from_db() + self.assertTrue(self.build_1.success) + self.assertEqual(self.build_1.error, '') + self.assertEqual(self.build_1.state, BUILD_STATE_CLONING) + + # Build with default time finished + self.build_2.refresh_from_db() + self.assertFalse(self.build_2.success) + self.assertNotEqual(self.build_2.error, '') + self.assertEqual(self.build_2.state, BUILD_STATE_FINISHED) + + # Build with custom time not finished + self.build_3.refresh_from_db() + self.assertTrue(self.build_3.success) + self.assertEqual(self.build_3.error, '') + self.assertEqual(self.build_3.state, BUILD_STATE_TRIGGERED)