diff --git a/docs/faq.rst b/docs/faq.rst index c6ad95f4690..0410cc65a13 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -104,7 +104,7 @@ If you add a subproject to a project, that documentation will also be served under the parent project's subdomain. For example, -Kombu is a subproject of celery, +Kombu is a subproject of Celery, so you can access it on the `celery.readthedocs.io` domain: http://celery.readthedocs.io/projects/kombu/en/latest/ @@ -204,4 +204,4 @@ file* field. What commit of Read the Docs is in production? ---------------------------------------------- -We deploy readthedocs.org from the `rel` branch in our GitHub repository. You can see the latest commits that have been deployed by looking on GitHub: https://github.com/rtfd/readthedocs.org/commits/rel \ No newline at end of file +We deploy readthedocs.org from the `rel` branch in our GitHub repository. You can see the latest commits that have been deployed by looking on GitHub: https://github.com/rtfd/readthedocs.org/commits/rel diff --git a/docs/index.rst b/docs/index.rst index 10675f8321f..24ead577682 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -92,18 +92,18 @@ Information about development is also available: :maxdepth: 2 :caption: Developer Documentation - install - api/index changelog + install + architecture tests docs - architecture development/standards development/buildenvironments symlinks settings i18n issue-labels + api/index .. _business-docs: diff --git a/readthedocs/__init__.py b/readthedocs/__init__.py index e69de29bb2d..1343a18d341 100644 --- a/readthedocs/__init__.py +++ b/readthedocs/__init__.py @@ -0,0 +1,4 @@ +"""Read the Docs""" + +# Import the Celery application before anything else happens +from readthedocs.worker import app # noqa diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py index f3927178987..10d8a9e5aa6 100644 --- a/readthedocs/builds/models.py +++ b/readthedocs/builds/models.py @@ -155,7 +155,7 @@ def save(self, *args, **kwargs): # pylint: disable=arguments-differ try: self.project.sync_supported_versions() except Exception: - log.error('failed to sync supported versions', exc_info=True) + log.exception('failed to sync supported versions') broadcast(type='app', task=tasks.symlink_project, args=[self.project.pk]) return obj @@ -224,7 +224,7 @@ def clean_build_path(self): path, self)) rmtree(path) except OSError: - log.error('Build path cleanup failed', exc_info=True) + log.exception('Build path cleanup failed') def get_github_url(self, docroot, filename, source_suffix='.rst', action='view'): """ diff --git a/readthedocs/core/management/commands/reindex_elasticsearch.py b/readthedocs/core/management/commands/reindex_elasticsearch.py index 731dc861af6..a2bce6df840 100644 --- a/readthedocs/core/management/commands/reindex_elasticsearch.py +++ b/readthedocs/core/management/commands/reindex_elasticsearch.py @@ -53,4 +53,4 @@ def handle(self, *args, **options): update_search(version.pk, commit, delete_non_commit_files=False) except Exception: - log.error('Reindex failed for %s', version, exc_info=True) + log.exception('Reindex failed for {}'.format(version)) diff --git a/readthedocs/core/management/commands/set_metadata.py b/readthedocs/core/management/commands/set_metadata.py index 2f8014cf828..dbefcddbdd0 100644 --- a/readthedocs/core/management/commands/set_metadata.py +++ b/readthedocs/core/management/commands/set_metadata.py @@ -23,4 +23,4 @@ def handle(self, *args, **options): try: broadcast(type='app', task=tasks.update_static_metadata, args=[p.pk]) except Exception: - log.error('Build failed for %s', p, exc_info=True) + log.exception('Build failed for %s', p) diff --git a/readthedocs/core/management/commands/update_api.py b/readthedocs/core/management/commands/update_api.py index 27d176f5ebb..e8433513bdd 100644 --- a/readthedocs/core/management/commands/update_api.py +++ b/readthedocs/core/management/commands/update_api.py @@ -33,4 +33,5 @@ def handle(self, *args, **options): project_data = api.project(slug).get() p = APIProject(**project_data) log.info("Building %s", p) - tasks.update_docs.run(pk=p.pk, docker=docker) + update_docs = tasks.UpdateDocsTask() + update_docs.run(pk=p.pk, docker=docker) diff --git a/readthedocs/core/management/commands/update_repos.py b/readthedocs/core/management/commands/update_repos.py index 142ad19e5e2..536e4c018e2 100644 --- a/readthedocs/core/management/commands/update_repos.py +++ b/readthedocs/core/management/commands/update_repos.py @@ -54,9 +54,11 @@ def handle(self, *args, **options): for version in Version.objects.filter(project__slug=slug, active=True, uploaded=False): - tasks.update_docs.run(pk=version.project_id, - record=False, - version_pk=version.pk) + tasks.UpdateDocsTask().run( + pk=version.project_id, + record=False, + version_pk=version.pk + ) else: p = Project.all_objects.get(slug=slug) log.info("Building %s", p) @@ -66,12 +68,17 @@ def handle(self, *args, **options): log.info("Updating all versions") for version in Version.objects.filter(active=True, uploaded=False): - tasks.update_docs.run(pk=version.project_id, - record=record, - force=force, - version_pk=version.pk) + tasks.UpdateDocsTask().run( + pk=version.project_id, + record=record, + force=force, + version_pk=version.pk + ) else: log.info("Updating all docs") for project in Project.objects.all(): - tasks.update_docs.run(pk=project.pk, record=record, - force=force) + tasks.UpdateDocsTask().run( + pk=project.pk, + record=record, + force=force + ) diff --git a/readthedocs/core/management/commands/update_versions.py b/readthedocs/core/management/commands/update_versions.py index 88e8d7c410c..c3ae6fa8e1b 100644 --- a/readthedocs/core/management/commands/update_versions.py +++ b/readthedocs/core/management/commands/update_versions.py @@ -4,7 +4,7 @@ from django.core.management.base import BaseCommand from readthedocs.builds.models import Version -from readthedocs.projects.tasks import update_docs +from readthedocs.projects.tasks import UpdateDocsTask class Command(BaseCommand): @@ -13,5 +13,6 @@ class Command(BaseCommand): def handle(self, *args, **options): for version in Version.objects.filter(active=True, built=False): + update_docs = UpdateDocsTask() update_docs.run(version.project_id, record=False, version_pk=version.pk) diff --git a/readthedocs/core/tasks.py b/readthedocs/core/tasks.py index 7c57cd90a4f..e5b67355e3f 100644 --- a/readthedocs/core/tasks.py +++ b/readthedocs/core/tasks.py @@ -3,19 +3,20 @@ from __future__ import absolute_import import logging -from celery import task from django.conf import settings from django.core.mail import EmailMultiAlternatives from django.template.loader import get_template from django.template import TemplateDoesNotExist +from readthedocs.worker import app + log = logging.getLogger(__name__) EMAIL_TIME_LIMIT = 30 -@task(queue='web', time_limit=EMAIL_TIME_LIMIT) +@app.task(queue='web', time_limit=EMAIL_TIME_LIMIT) def send_email_task(recipient, subject, template, template_html, context=None): """Send multipart email diff --git a/readthedocs/core/utils/__init__.py b/readthedocs/core/utils/__init__.py index fca8e13f96f..80e5d90631b 100644 --- a/readthedocs/core/utils/__init__.py +++ b/readthedocs/core/utils/__init__.py @@ -49,9 +49,15 @@ def broadcast(type, task, args, kwargs=None, callback=None): # pylint: disable= task_sig = task.s(*args, **kwargs).set(queue=server) tasks.append(task_sig) if callback: - task_promise = chord(tasks)(callback).get() + task_promise = chord(tasks, callback).apply_async() else: - task_promise = group(*tasks).apply_async() + # Celery's Group class does some special handling when an iterable with + # len() == 1 is passed in. This will be hit if there is only one server + # defined in the above queue lists + if len(tasks) > 1: + task_promise = group(*tasks).apply_async() + else: + task_promise = group(tasks).apply_async() return task_promise @@ -77,7 +83,7 @@ def trigger_build(project, version=None, record=True, force=False, basic=False): will be prefixed with ``build-`` to unify build queue names. """ # Avoid circular import - from readthedocs.projects.tasks import update_docs + from readthedocs.projects.tasks import UpdateDocsTask from readthedocs.builds.models import Build if project.skip: @@ -121,6 +127,7 @@ def trigger_build(project, version=None, record=True, force=False, basic=False): options['soft_time_limit'] = time_limit options['time_limit'] = int(time_limit * 1.2) + update_docs = UpdateDocsTask() update_docs.apply_async(kwargs=kwargs, **options) return build diff --git a/readthedocs/core/utils/tasks/retrieve.py b/readthedocs/core/utils/tasks/retrieve.py index f33afab8094..9da3c581601 100644 --- a/readthedocs/core/utils/tasks/retrieve.py +++ b/readthedocs/core/utils/tasks/retrieve.py @@ -1,7 +1,6 @@ """Utilities for retrieving task data.""" from __future__ import absolute_import -from djcelery import celery as celery_app from celery.result import AsyncResult @@ -20,6 +19,8 @@ def get_task_data(task_id): meta data has no ``'task_name'`` key set. """ + from readthedocs.worker import app + result = AsyncResult(task_id) state, info = result.state, result.info if state == 'PENDING': @@ -27,7 +28,7 @@ def get_task_data(task_id): if 'task_name' not in info: raise TaskNotFound(task_id) try: - task = celery_app.tasks[info['task_name']] + task = app.tasks[info['task_name']] except KeyError: raise TaskNotFound(task_id) return task, state, info diff --git a/readthedocs/core/views/hooks.py b/readthedocs/core/views/hooks.py index 3525fb88177..2a438f27da8 100644 --- a/readthedocs/core/views/hooks.py +++ b/readthedocs/core/views/hooks.py @@ -178,7 +178,7 @@ def github_build(request): # noqa: D205 ssh_search_url = ssh_url.replace('git@', '').replace('.git', '') branches = [data['ref'].replace('refs/heads/', '')] except (ValueError, TypeError, KeyError): - log.error('Invalid GitHub webhook payload', exc_info=True) + log.exception('Invalid GitHub webhook payload') return HttpResponse('Invalid request', status=400) try: repo_projects = get_project_from_url(http_search_url) @@ -198,7 +198,7 @@ def github_build(request): # noqa: D205 projects = repo_projects | ssh_projects return _build_url(http_search_url, projects, branches) except NoProjectException: - log.error('Project match not found: url=%s', http_search_url) + log.exception('Project match not found: url=%s', http_search_url) return HttpResponseNotFound('Project not found') else: return HttpResponse('Method not allowed, POST is required', status=405) @@ -222,7 +222,7 @@ def gitlab_build(request): # noqa: D205 search_url = re.sub(r'^https?://(.*?)(?:\.git|)$', '\\1', url) branches = [data['ref'].replace('refs/heads/', '')] except (ValueError, TypeError, KeyError): - log.error('Invalid GitLab webhook payload', exc_info=True) + log.exception('Invalid GitLab webhook payload') return HttpResponse('Invalid request', status=400) log.info( 'GitLab webhook search: url=%s branches=%s', @@ -281,7 +281,7 @@ def bitbucket_build(request): data['repository']['full_name'] ) except (TypeError, ValueError, KeyError): - log.error('Invalid Bitbucket webhook payload', exc_info=True) + log.exception('Invalid Bitbucket webhook payload') return HttpResponse('Invalid request', status=400) log.info( @@ -320,7 +320,7 @@ def generic_build(request, project_id_or_slug=None): try: project = Project.objects.get(slug=project_id_or_slug) except (Project.DoesNotExist, ValueError): - log.error( + log.exception( "(Incoming Generic Build) Repo not found: %s", project_id_or_slug) return HttpResponseNotFound( diff --git a/readthedocs/doc_builder/environments.py b/readthedocs/doc_builder/environments.py index 3fa97b0d036..e2f8ed6707c 100644 --- a/readthedocs/doc_builder/environments.py +++ b/readthedocs/doc_builder/environments.py @@ -418,10 +418,17 @@ def update_build(self, state=None): # BuildEnvironmentException or BuildEnvironmentWarning if isinstance(self.failure, (BuildEnvironmentException, BuildEnvironmentWarning)): - self.build['error'] = str(self.failure) + self.build['error'] = ugettext_noop( + "A failure in building the documentation as occured: {}".format( + str(self.failure) + ) + ) else: self.build['error'] = ugettext_noop( - "An unexpected error occurred") + "A failure in our code has occured. The failure is: {}".format( + str(self.failure) + ) + ) # Attempt to stop unicode errors on build reporting for key, val in list(self.build.items()): @@ -433,7 +440,7 @@ def update_build(self, state=None): except HttpClientError as e: log.error("Unable to post a new build: %s", e.content) except Exception: - log.error("Unknown build exception", exc_info=True) + log.exception("Unknown build exception") class LocalEnvironment(BuildEnvironment): diff --git a/readthedocs/oauth/services/github.py b/readthedocs/oauth/services/github.py index a0c630b1a3a..9472f20825f 100644 --- a/readthedocs/oauth/services/github.py +++ b/readthedocs/oauth/services/github.py @@ -289,5 +289,5 @@ def get_token_for_project(cls, project, force_local=False): if tokens.exists(): token = tokens[0].token except Exception: - log.error('Failed to get token for project', exc_info=True) + log.exception('Failed to get token for project') return token diff --git a/readthedocs/oauth/tasks.py b/readthedocs/oauth/tasks.py index b345d443046..7f95f9610ea 100644 --- a/readthedocs/oauth/tasks.py +++ b/readthedocs/oauth/tasks.py @@ -2,7 +2,6 @@ from __future__ import absolute_import from django.contrib.auth.models import User -from djcelery import celery as celery_app from readthedocs.core.utils.tasks import PublicTask from readthedocs.core.utils.tasks import permission_check @@ -22,4 +21,4 @@ def run_public(self, user_id): service.sync() -sync_remote_repositories = celery_app.tasks[SyncRemoteRepositories.name] +sync_remote_repositories = SyncRemoteRepositories() diff --git a/readthedocs/projects/__init__.py b/readthedocs/projects/__init__.py index e69de29bb2d..ff5ded49b17 100644 --- a/readthedocs/projects/__init__.py +++ b/readthedocs/projects/__init__.py @@ -0,0 +1 @@ +default_app_config = 'readthedocs.projects.apps.ProjectsConfig' diff --git a/readthedocs/projects/apps.py b/readthedocs/projects/apps.py new file mode 100644 index 00000000000..71111f4e8d0 --- /dev/null +++ b/readthedocs/projects/apps.py @@ -0,0 +1,12 @@ +"""Project app config""" + +from django.apps import AppConfig + + +class ProjectsConfig(AppConfig): + name = 'readthedocs.projects' + + def ready(self): + from readthedocs.projects.tasks import UpdateDocsTask + from readthedocs.worker import app + app.tasks.register(UpdateDocsTask) diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index f7c3adb32d5..9dc5a682367 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -315,29 +315,29 @@ def save(self, *args, **kwargs): # pylint: disable=arguments-differ latest.identifier = self.default_branch latest.save() except Exception: - log.error('Failed to update latest identifier', exc_info=True) + log.exception('Failed to update latest identifier') # Add exceptions here for safety try: self.sync_supported_versions() except Exception: - log.error('failed to sync supported versions', exc_info=True) + log.exception('failed to sync supported versions') try: if not first_save: broadcast(type='app', task=tasks.symlink_project, args=[self.pk]) except Exception: - log.error('failed to symlink project', exc_info=True) + log.exception('failed to symlink project') try: if not first_save: broadcast(type='app', task=tasks.update_static_metadata, args=[self.pk]) except Exception: - log.error('failed to update static metadata', exc_info=True) + log.exception('failed to update static metadata') try: branch = self.default_branch or self.vcs_repo().fallback_branch if not self.versions.filter(slug=LATEST).exists(): self.versions.create_latest(identifier=branch) except Exception: - log.error('Error creating default branches', exc_info=True) + log.exception('Error creating default branches') def get_absolute_url(self): return reverse('projects_detail', args=[self.slug]) diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index a52fa80c3af..1a75ad8d07c 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -16,12 +16,11 @@ import requests from builtins import str -from celery import task, Task +from celery import Task from celery.exceptions import SoftTimeLimitExceeded from django.conf import settings from django.core.urlresolvers import reverse from django.utils.translation import ugettext_lazy as _ -from djcelery import celery as celery_app from readthedocs_build.config import ConfigError from slumber.exceptions import HttpClientError @@ -53,6 +52,7 @@ from readthedocs.search.parse_json import process_all_json_files from readthedocs.search.utils import process_mkdocs_json from readthedocs.vcs_support import utils as vcs_support_utils +from readthedocs.worker import app log = logging.getLogger(__name__) @@ -80,7 +80,7 @@ class UpdateDocsTask(Task): max_retries = 5 default_retry_delay = (7 * 60) - name = 'update_docs' + name = __name__ + '.update_docs' def __init__(self, build_env=None, python_env=None, config=None, force=False, search=True, localmedia=True, @@ -108,20 +108,46 @@ def _log(self, msg): version=self.version.slug, msg=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, **__): - # pylint: disable=arguments-differ - self.project = self.get_project(pk) - self.version = self.get_version(self.project, version_pk) - self.build = self.get_build(build_pk) - self.build_search = search - self.build_localmedia = localmedia - self.build_force = force - self.config = None + """ + Run a documentation build. - setup_successful = self.run_setup(record=record) - if setup_successful: - self.run_build(record=record, docker=docker) + This is fully wrapped in exception handling to account for a number of failure cases. + """ + try: + self.project = self.get_project(pk) + self.version = self.get_version(self.project, version_pk) + self.build = self.get_build(build_pk) + self.build_search = search + self.build_localmedia = localmedia + self.build_force = force + self.config = None + + setup_successful = self.run_setup(record=record) + if setup_successful: + self.run_build(record=record, docker=docker) + failure = self.setup_env.failure or self.build_env.failure + except Exception as e: # noqa + log.exception('Top-level build exception has been raised') + failure = str(e) + + # **Always** report build status. + # This can still fail if the API Is totally down, but should catch more failures + result = {} + if hasattr(self, 'build'): + self.build['state'] = BUILD_STATE_FINISHED + if failure: + self.build['error'] = 'Unknown setup failure: {}'.format(failure) + self.build['success'] = False + result = api_v2.build(build_pk).patch(self.build) + else: + build_updates = {'state': BUILD_STATE_FINISHED, + 'success': False, + 'error': 'Unknown setup failure: {}'.format(failure)} + result = api_v2.build(build_pk).patch(build_updates) + return result def run_setup(self, record=True): """Run setup in the local environment. @@ -166,7 +192,6 @@ def run_setup(self, record=True): if not isinstance(self.setup_env.failure, vcs_support_utils.LockTimeout): self.send_notifications() - self.setup_env.update_build(state=BUILD_STATE_FINISHED) return False if self.setup_env.successful and not self.project.has_valid_clone: @@ -230,14 +255,11 @@ def run_build(self, docker=False, record=True): self.send_notifications() build_complete.send(sender=Build, build=self.build_env.build) - self.build_env.update_build(state=BUILD_STATE_FINISHED) - @staticmethod def get_project(project_pk): """Get project from API""" project_data = api_v2.project(project_pk).get() - project = APIProject(**project_data) - return project + return APIProject(**project_data) @staticmethod def get_version(project, version_pk): @@ -490,10 +512,7 @@ def send_notifications(self): send_notifications.delay(self.version.pk, build_pk=self.build['id']) -update_docs = celery_app.tasks[UpdateDocsTask.name] - - -@task() +@app.task() def update_imported_docs(version_pk): """ Check out or update the given project's repository @@ -572,7 +591,7 @@ def update_imported_docs(version_pk): # Web tasks -@task(queue='web') +@app.task(queue='web') 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 @@ -604,7 +623,7 @@ def sync_files(project_pk, version_pk, hostname=None, html=False, update_static_metadata(project_pk) -@task(queue='web') +@app.task(queue='web') def move_files(version_pk, hostname, html=False, localmedia=False, search=False, pdf=False, epub=False): """Task to move built documentation to web servers @@ -624,7 +643,7 @@ def move_files(version_pk, hostname, html=False, localmedia=False, search=False, """ version = Version.objects.get(pk=version_pk) log.debug(LOG_TEMPLATE.format(project=version.project.slug, version=version.slug, - msg='Moving files: {}'.format(locals()))) + msg='Moving files')) if html: from_path = version.project.artifact_path( @@ -670,7 +689,7 @@ def move_files(version_pk, hostname, html=False, localmedia=False, search=False, Syncer.copy(from_path, to_path, host=hostname) -@task(queue='web') +@app.task(queue='web') def update_search(version_pk, commit, delete_non_commit_files=True): """Task to update search indexes @@ -705,7 +724,7 @@ def update_search(version_pk, commit, delete_non_commit_files=True): ) -@task(queue='web') +@app.task(queue='web') def symlink_project(project_pk): project = Project.objects.get(pk=project_pk) for symlink in [PublicSymlink, PrivateSymlink]: @@ -713,7 +732,7 @@ def symlink_project(project_pk): sym.run() -@task(queue='web') +@app.task(queue='web') def symlink_domain(project_pk, domain_pk, delete=False): project = Project.objects.get(pk=project_pk) domain = Domain.objects.get(pk=domain_pk) @@ -725,7 +744,7 @@ def symlink_domain(project_pk, domain_pk, delete=False): sym.symlink_cnames(domain) -@task(queue='web') +@app.task(queue='web') def symlink_subproject(project_pk): project = Project.objects.get(pk=project_pk) for symlink in [PublicSymlink, PrivateSymlink]: @@ -733,7 +752,7 @@ def symlink_subproject(project_pk): sym.symlink_subprojects() -@task(queue='web') +@app.task(queue='web') def fileify(version_pk, commit): """ Create ImportedFile objects for all of a version's files. @@ -809,7 +828,7 @@ def _manage_imported_files(version, path, commit): purge(cdn_ids[version.project.slug], changed_files) -@task(queue='web') +@app.task(queue='web') def send_notifications(version_pk, build_pk): version = Version.objects.get(pk=version_pk) build = Build.objects.get(pk=build_pk) @@ -878,7 +897,7 @@ def webhook_notification(version, build, hook_url): requests.post(hook_url, data=data) -@task(queue='web') +@app.task(queue='web') def update_static_metadata(project_pk, path=None): """Update static metadata JSON file @@ -924,7 +943,7 @@ def update_static_metadata(project_pk, path=None): # Random Tasks -@task() +@app.task() def remove_dir(path): """ Remove a directory on the build/celery server. @@ -936,7 +955,7 @@ def remove_dir(path): shutil.rmtree(path, ignore_errors=True) -@task() +@app.task() def clear_artifacts(version_pk): """Remove artifacts from the web servers""" version = Version.objects.get(pk=version_pk) @@ -946,7 +965,7 @@ def clear_artifacts(version_pk): clear_html_artifacts(version) -@task() +@app.task() def clear_pdf_artifacts(version): if isinstance(version, int): version = Version.objects.get(pk=version) @@ -954,7 +973,7 @@ def clear_pdf_artifacts(version): type_='pdf', version_slug=version.slug)) -@task() +@app.task() def clear_epub_artifacts(version): if isinstance(version, int): version = Version.objects.get(pk=version) @@ -962,7 +981,7 @@ def clear_epub_artifacts(version): type_='epub', version_slug=version.slug)) -@task() +@app.task() def clear_htmlzip_artifacts(version): if isinstance(version, int): version = Version.objects.get(pk=version) @@ -970,14 +989,14 @@ def clear_htmlzip_artifacts(version): type_='htmlzip', version_slug=version.slug)) -@task() +@app.task() def clear_html_artifacts(version): if isinstance(version, int): version = Version.objects.get(pk=version) remove_dir(version.project.rtd_build_path(version=version.slug)) -@task(queue='web') +@app.task(queue='web') def sync_callback(_, version_pk, commit, *args, **kwargs): """ This will be called once the sync_files tasks are done. diff --git a/readthedocs/projects/utils.py b/readthedocs/projects/utils.py index 86e2040d03c..2683ec59cfb 100644 --- a/readthedocs/projects/utils.py +++ b/readthedocs/projects/utils.py @@ -84,7 +84,7 @@ def run(*commands): command = ' '.join(command) except TypeError: run_command = command - log.info('Running command: cwd=%s command=%s', cwd, command) + log.debug('Running command: cwd=%s command=%s', cwd, command) try: p = subprocess.Popen( run_command, @@ -100,7 +100,7 @@ def run(*commands): out = '' err = traceback.format_exc() ret = -1 - log.error("Command failed", exc_info=True) + log.exception("Command failed") return (ret, out, err) diff --git a/readthedocs/restapi/client.py b/readthedocs/restapi/client.py index 3ebb6b2a82e..c9cb90307ce 100644 --- a/readthedocs/restapi/client.py +++ b/readthedocs/restapi/client.py @@ -4,7 +4,7 @@ import logging from slumber import API, serialize -from requests import Session +import requests from django.conf import settings from rest_framework.renderers import JSONRenderer from rest_framework.parsers import JSONParser @@ -32,8 +32,10 @@ def dumps(self, data): def setup_api(): - session = Session() + session = requests.Session() session.headers.update({'Host': PRODUCTION_DOMAIN}) + retry_adapter = requests.adapters.HTTPAdapter(max_retries=3) + session.mount(API_HOST, retry_adapter) api_config = { 'base_url': '%s/api/v2/' % API_HOST, 'serializer': serialize.Serializer( diff --git a/readthedocs/rtd_tests/tests/projects/test_admin_actions.py b/readthedocs/rtd_tests/tests/projects/test_admin_actions.py index b0f5526b331..7bda4501bf7 100644 --- a/readthedocs/rtd_tests/tests/projects/test_admin_actions.py +++ b/readthedocs/rtd_tests/tests/projects/test_admin_actions.py @@ -57,9 +57,10 @@ def test_project_ban_multiple_owners(self): self.assertFalse(self.project.users.filter(profile__banned=True).exists()) self.assertEqual(self.project.users.filter(profile__banned=False).count(), 2) - @mock.patch('readthedocs.projects.admin.remove_dir') - def test_project_delete(self, remove_dir): + @mock.patch('readthedocs.projects.admin.broadcast') + def test_project_delete(self, broadcast): """Test project and artifacts are removed""" + from readthedocs.projects.tasks import remove_dir action_data = { ACTION_CHECKBOX_NAME: [self.project.pk], 'action': 'delete_selected', @@ -71,8 +72,8 @@ def test_project_delete(self, remove_dir): action_data ) self.assertFalse(Project.objects.filter(pk=self.project.pk).exists()) - remove_dir.s.assert_has_calls([ + broadcast.assert_has_calls([ mock.call( - self.project.doc_path, + type='app', task=remove_dir, args=[self.project.doc_path] ), ]) diff --git a/readthedocs/rtd_tests/tests/test_celery.py b/readthedocs/rtd_tests/tests/test_celery.py index d73c2c58fca..49db6b96dd0 100644 --- a/readthedocs/rtd_tests/tests/test_celery.py +++ b/readthedocs/rtd_tests/tests/test_celery.py @@ -64,56 +64,56 @@ def test_clear_artifacts(self): self.assertTrue(result.successful()) self.assertFalse(exists(directory)) - @patch('readthedocs.projects.tasks.UpdateDocsTask.build_docs', - new=MagicMock) - @patch('readthedocs.projects.tasks.UpdateDocsTask.setup_vcs', - new=MagicMock) + @patch('readthedocs.projects.tasks.UpdateDocsTask.setup_environment', new=MagicMock) + @patch('readthedocs.projects.tasks.UpdateDocsTask.build_docs', new=MagicMock) + @patch('readthedocs.projects.tasks.UpdateDocsTask.setup_vcs', new=MagicMock) def test_update_docs(self): build = get(Build, project=self.project, version=self.project.versions.first()) with mock_api(self.repo) as mapi: - result = tasks.update_docs.delay( + update_docs = tasks.UpdateDocsTask() + result = update_docs.delay( self.project.pk, build_pk=build.pk, record=False, intersphinx=False) self.assertTrue(result.successful()) - @patch('readthedocs.projects.tasks.UpdateDocsTask.build_docs', - new=MagicMock) + @patch('readthedocs.projects.tasks.UpdateDocsTask.setup_environment', new=MagicMock) + @patch('readthedocs.projects.tasks.UpdateDocsTask.build_docs', new=MagicMock) + @patch('readthedocs.doc_builder.environments.BuildEnvironment.update_build', new=MagicMock) @patch('readthedocs.projects.tasks.UpdateDocsTask.setup_vcs') - @patch('readthedocs.doc_builder.environments.BuildEnvironment.update_build') - def test_update_docs_unexpected_setup_exception(self, mock_update_build, mock_setup_vcs): + def test_update_docs_unexpected_setup_exception(self, mock_setup_vcs): exc = Exception() mock_setup_vcs.side_effect = exc build = get(Build, project=self.project, version=self.project.versions.first()) with mock_api(self.repo) as mapi: - result = tasks.update_docs.delay( + update_docs = tasks.UpdateDocsTask() + result = update_docs.delay( self.project.pk, build_pk=build.pk, record=False, intersphinx=False) self.assertTrue(result.successful()) - mock_update_build.assert_called_once_with(state=BUILD_STATE_FINISHED) + @patch('readthedocs.projects.tasks.UpdateDocsTask.setup_environment', new=MagicMock) + @patch('readthedocs.projects.tasks.UpdateDocsTask.setup_vcs', new=MagicMock) + @patch('readthedocs.doc_builder.environments.BuildEnvironment.update_build', new=MagicMock) @patch('readthedocs.projects.tasks.UpdateDocsTask.build_docs') - @patch('readthedocs.projects.tasks.UpdateDocsTask.setup_vcs', - new=MagicMock) - @patch('readthedocs.doc_builder.environments.BuildEnvironment.update_build') - def test_update_docs_unexpected_build_exception(self, mock_update_build, mock_build_docs): + def test_update_docs_unexpected_build_exception(self, mock_build_docs): exc = Exception() mock_build_docs.side_effect = exc build = get(Build, project=self.project, version=self.project.versions.first()) with mock_api(self.repo) as mapi: - result = tasks.update_docs.delay( + update_docs = tasks.UpdateDocsTask() + result = update_docs.delay( self.project.pk, build_pk=build.pk, record=False, intersphinx=False) self.assertTrue(result.successful()) - mock_update_build.assert_called_with(state=BUILD_STATE_FINISHED) def test_update_imported_doc(self): with mock_api(self.repo): diff --git a/readthedocs/rtd_tests/tests/test_core_utils.py b/readthedocs/rtd_tests/tests/test_core_utils.py index b48795e8807..dc3424b93e8 100644 --- a/readthedocs/rtd_tests/tests/test_core_utils.py +++ b/readthedocs/rtd_tests/tests/test_core_utils.py @@ -18,12 +18,12 @@ def setUp(self): self.project = get(Project, container_time_limit=None) self.version = get(Version, project=self.project) - @mock.patch('readthedocs.projects.tasks.update_docs') + @mock.patch('readthedocs.projects.tasks.UpdateDocsTask') def test_trigger_build_time_limit(self, update_docs): """Pass of time limit""" trigger_build(project=self.project, version=self.version) - update_docs.assert_has_calls([ - mock.call.apply_async( + update_docs().apply_async.assert_has_calls([ + mock.call( time_limit=720, soft_time_limit=600, queue=mock.ANY, @@ -38,13 +38,13 @@ def test_trigger_build_time_limit(self, update_docs): ) ]) - @mock.patch('readthedocs.projects.tasks.update_docs') + @mock.patch('readthedocs.projects.tasks.UpdateDocsTask') def test_trigger_build_invalid_time_limit(self, update_docs): """Time limit as string""" self.project.container_time_limit = '200s' trigger_build(project=self.project, version=self.version) - update_docs.assert_has_calls([ - mock.call.apply_async( + update_docs().apply_async.assert_has_calls([ + mock.call( time_limit=720, soft_time_limit=600, queue=mock.ANY, @@ -59,13 +59,13 @@ def test_trigger_build_invalid_time_limit(self, update_docs): ) ]) - @mock.patch('readthedocs.projects.tasks.update_docs') + @mock.patch('readthedocs.projects.tasks.UpdateDocsTask') def test_trigger_build_rounded_time_limit(self, update_docs): """Time limit should round down""" self.project.container_time_limit = 3 trigger_build(project=self.project, version=self.version) - update_docs.assert_has_calls([ - mock.call.apply_async( + update_docs().apply_async.assert_has_calls([ + mock.call( time_limit=3, soft_time_limit=3, queue=mock.ANY, diff --git a/readthedocs/rtd_tests/tests/test_privacy_urls.py b/readthedocs/rtd_tests/tests/test_privacy_urls.py index b9fa7c0eaeb..e992a55ae1c 100644 --- a/readthedocs/rtd_tests/tests/test_privacy_urls.py +++ b/readthedocs/rtd_tests/tests/test_privacy_urls.py @@ -1,22 +1,23 @@ from __future__ import absolute_import from __future__ import print_function -from builtins import object import re +from builtins import object from django.contrib.admindocs.views import extract_views_from_urlpatterns from django.test import TestCase from django.core.urlresolvers import reverse +from django_dynamic_fixture import get +import mock +from taggit.models import Tag from readthedocs.builds.models import Build, VersionAlias, BuildCommandResult from readthedocs.comments.models import DocumentComment, NodeSnapshot +from readthedocs.core.utils.tasks import TaskNoPermission from readthedocs.integrations.models import HttpExchange, Integration from readthedocs.projects.models import Project, Domain from readthedocs.oauth.models import RemoteRepository, RemoteOrganization from readthedocs.rtd_tests.utils import create_user -from django_dynamic_fixture import get -from taggit.models import Tag - class URLAccessMixin(object): @@ -341,8 +342,10 @@ def setUp(self): class APIUnauthAccessTest(APIMixin, TestCase): - def test_api_urls(self): + @mock.patch('readthedocs.restapi.views.task_views.get_public_task_data') + def test_api_urls(self, get_public_task_data): from readthedocs.restapi.urls import urlpatterns + get_public_task_data.side_effect = TaskNoPermission('Nope') self._test_url(urlpatterns) def login(self): diff --git a/readthedocs/search/utils.py b/readthedocs/search/utils.py index 07eb38cd1f8..385e1110804 100644 --- a/readthedocs/search/utils.py +++ b/readthedocs/search/utils.py @@ -266,7 +266,7 @@ def parse_mkdocs_sections(content): # we're unsure which exceptions can be raised # pylint: disable=bare-except except: - log.error('Failed indexing', exc_info=True) + log.exception('Failed indexing') def parse_sections(documentation_type, content): diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index d595f49e430..a46ccaefa75 100644 --- a/readthedocs/settings/base.py +++ b/readthedocs/settings/base.py @@ -4,8 +4,6 @@ from __future__ import absolute_import import os -import djcelery - from readthedocs.core.settings import Settings try: @@ -15,8 +13,6 @@ donate = False -djcelery.setup_loader() - _ = gettext = lambda s: s @@ -85,9 +81,6 @@ def INSTALLED_APPS(self): # noqa 'django_extensions', 'messages_extends', - # Celery bits - 'djcelery', - # daniellindsleyrocksdahouse 'haystack', 'tastypie', diff --git a/readthedocs/vcs_support/test.py b/readthedocs/vcs_support/tests.py similarity index 100% rename from readthedocs/vcs_support/test.py rename to readthedocs/vcs_support/tests.py diff --git a/readthedocs/vcs_support/utils.py b/readthedocs/vcs_support/utils.py index f4e2018a010..5dd59c80f1b 100644 --- a/readthedocs/vcs_support/utils.py +++ b/readthedocs/vcs_support/utils.py @@ -57,8 +57,7 @@ def __exit__(self, exc, value, tb): log.info("Lock (%s): Releasing", self.name) os.remove(self.fpath) except OSError: - log.error("Lock (%s): Failed to release, ignoring...", self.name, - exc_info=True) + log.exception("Lock (%s): Failed to release, ignoring...", self.name) class NonBlockingLock(object): diff --git a/readthedocs/worker.py b/readthedocs/worker.py new file mode 100644 index 00000000000..e04d30d6dbb --- /dev/null +++ b/readthedocs/worker.py @@ -0,0 +1,19 @@ +"""Celery worker application instantiation""" + +from __future__ import absolute_import, unicode_literals + +import os + +from celery import Celery + + +def create_application(): + """Create a Celery application using Django settings""" + os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'readthedocs.settings.dev') + application = Celery('readthedocs') + application.config_from_object('django.conf:settings') + application.autodiscover_tasks(None) + return application + + +app = create_application() # pylint: disable=invalid-name diff --git a/requirements/pip.txt b/requirements/pip.txt index caa6b917067..2a56390d8ae 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -26,9 +26,8 @@ lxml==3.3.5 defusedxml==0.5.0 # Basic tools -redis==2.10.3 -celery==3.1.23 -django-celery==3.2.1 +redis==2.10.6 +celery==4.1.0 django-allauth==0.32.0 dnspython==1.15.0