From 9681ebf0bb5d6cc53d22b0d5b5462b72bb88b979 Mon Sep 17 00:00:00 2001 From: shubham76 Date: Sat, 14 Apr 2018 03:20:46 +0530 Subject: [PATCH 1/8] Refactoring a single function to test --- readthedocs/builds/tasks.py | 84 +++++++++++++++++++++++++++++++++++ readthedocs/projects/tasks.py | 30 +------------ 2 files changed, 85 insertions(+), 29 deletions(-) create mode 100644 readthedocs/builds/tasks.py diff --git a/readthedocs/builds/tasks.py b/readthedocs/builds/tasks.py new file mode 100644 index 00000000000..6a85a27fb67 --- /dev/null +++ b/readthedocs/builds/tasks.py @@ -0,0 +1,84 @@ + +from __future__ import absolute_import + +import datetime +import hashlib +import json +import logging +import os +import shutil +import socket +from collections import defaultdict + +import requests +from builtins import str +from celery import Task +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 + +from readthedocs.builds.constants import (LATEST, + BUILD_STATE_CLONING, + BUILD_STATE_INSTALLING, + BUILD_STATE_BUILDING, + BUILD_STATE_FINISHED) +from readthedocs.builds.models import Build, Version, APIVersion +from readthedocs.builds.signals import build_complete +from readthedocs.builds.syncers import Syncer +from readthedocs.cdn.purge import purge +from readthedocs.core.resolver import resolve_path +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 (LocalBuildEnvironment, + DockerBuildEnvironment) +from readthedocs.doc_builder.exceptions import BuildEnvironmentError +from readthedocs.doc_builder.loader import get_builder_class +from readthedocs.doc_builder.python_environments import Virtualenv, Conda +from readthedocs.projects.models import APIProject +from readthedocs.restapi.client import api as api_v2 +from readthedocs.restapi.utils import index_search_request +from readthedocs.search.parse_json import process_all_json_files +from readthedocs.vcs_support import utils as vcs_support_utils +from readthedocs.worker import app + + +log = logging.getLogger(__name__) + +HTML_ONLY = getattr(settings, 'HTML_ONLY_PROJECTS', ()) + + +@app.task(queue='web') +def fileify(version_pk, commit): + """ + Create ImportedFile objects for all of a version's files. + + This is so we have an idea of what files we have in the database. + """ + version = Version.objects.get(pk=version_pk) + project = version.project + + if not commit: + log.info(LOG_TEMPLATE + .format(project=project.slug, version=version.slug, + msg=('Imported File not being built because no commit ' + 'information'))) + return + + path = project.rtd_build_path(version.slug) + if path: + log.info(LOG_TEMPLATE + .format(project=version.project.slug, version=version.slug, + msg='Creating ImportedFiles')) + _manage_imported_files(version, path, commit) + else: + log.info(LOG_TEMPLATE + .format(project=project.slug, version=version.slug, + msg='No ImportedFile files')) + + diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 63e33a6fb4f..1cd3ec7692b 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -39,6 +39,7 @@ from readthedocs.builds.models import Build, Version, APIVersion from readthedocs.builds.signals import build_complete from readthedocs.builds.syncers import Syncer +from readthedocs.builds.tasks import fileify from readthedocs.cdn.purge import purge from readthedocs.core.resolver import resolve_path from readthedocs.core.symlink import PublicSymlink, PrivateSymlink @@ -859,35 +860,6 @@ def symlink_subproject(project_pk): sym.symlink_subprojects() -@app.task(queue='web') -def fileify(version_pk, commit): - """ - Create ImportedFile objects for all of a version's files. - - This is so we have an idea of what files we have in the database. - """ - version = Version.objects.get(pk=version_pk) - project = version.project - - if not commit: - log.info(LOG_TEMPLATE - .format(project=project.slug, version=version.slug, - msg=('Imported File not being built because no commit ' - 'information'))) - return - - path = project.rtd_build_path(version.slug) - if path: - log.info(LOG_TEMPLATE - .format(project=version.project.slug, version=version.slug, - msg='Creating ImportedFiles')) - _manage_imported_files(version, path, commit) - else: - log.info(LOG_TEMPLATE - .format(project=project.slug, version=version.slug, - msg='No ImportedFile files')) - - def _manage_imported_files(version, path, commit): """ Update imported files for version. From 53e311828467bdbc0a64897ecb69148f0e77165f Mon Sep 17 00:00:00 2001 From: shubham76 Date: Sun, 15 Apr 2018 00:32:53 +0530 Subject: [PATCH 2/8] first four functions moved and removed unnecessory imports --- readthedocs/builds/constants.py | 2 + readthedocs/builds/tasks.py | 172 ++++++++++++++++++++++++-------- readthedocs/projects/tasks.py | 89 +---------------- 3 files changed, 136 insertions(+), 127 deletions(-) diff --git a/readthedocs/builds/constants.py b/readthedocs/builds/constants.py index 590d9ca03b1..58b5d69f3fa 100644 --- a/readthedocs/builds/constants.py +++ b/readthedocs/builds/constants.py @@ -50,3 +50,5 @@ LATEST, STABLE, ) + +LOG_TEMPLATE = '(Build) [{project}:{version}] {msg}' diff --git a/readthedocs/builds/tasks.py b/readthedocs/builds/tasks.py index 6a85a27fb67..4d563d4783f 100644 --- a/readthedocs/builds/tasks.py +++ b/readthedocs/builds/tasks.py @@ -1,56 +1,65 @@ from __future__ import absolute_import -import datetime -import hashlib -import json import logging -import os -import shutil -import socket -from collections import defaultdict +import json import requests -from builtins import str -from celery import Task -from celery.exceptions import SoftTimeLimitExceeded from django.conf import settings + +from .constants import LOG_TEMPLATE 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 - -from readthedocs.builds.constants import (LATEST, - BUILD_STATE_CLONING, - BUILD_STATE_INSTALLING, - BUILD_STATE_BUILDING, - BUILD_STATE_FINISHED) -from readthedocs.builds.models import Build, Version, APIVersion -from readthedocs.builds.signals import build_complete -from readthedocs.builds.syncers import Syncer -from readthedocs.cdn.purge import purge -from readthedocs.core.resolver import resolve_path -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 (LocalBuildEnvironment, - DockerBuildEnvironment) -from readthedocs.doc_builder.exceptions import BuildEnvironmentError -from readthedocs.doc_builder.loader import get_builder_class -from readthedocs.doc_builder.python_environments import Virtualenv, Conda -from readthedocs.projects.models import APIProject -from readthedocs.restapi.client import api as api_v2 -from readthedocs.restapi.utils import index_search_request -from readthedocs.search.parse_json import process_all_json_files -from readthedocs.vcs_support import utils as vcs_support_utils +from readthedocs.builds.models import Build, Version, APIVersion from readthedocs.worker import app - log = logging.getLogger(__name__) -HTML_ONLY = getattr(settings, 'HTML_ONLY_PROJECTS', ()) +#copy of this function exists in projects/tasks.py +def _manage_imported_files(version, path, commit): + """ + Update imported files for version. + + :param version: Version instance + :param path: Path to search + :param commit: Commit that updated path + """ + changed_files = set() + for root, __, filenames in os.walk(path): + for filename in filenames: + dirpath = os.path.join(root.replace(path, '').lstrip('/'), + filename.lstrip('/')) + full_path = os.path.join(root, filename) + md5 = hashlib.md5(open(full_path, 'rb').read()).hexdigest() + try: + obj, __ = ImportedFile.objects.get_or_create( + project=version.project, + version=version, + path=dirpath, + name=filename, + ) + except ImportedFile.MultipleObjectsReturned: + log.warning('Error creating ImportedFile') + continue + if obj.md5 != md5: + obj.md5 = md5 + changed_files.add(dirpath) + if obj.commit != commit: + obj.commit = commit + obj.save() + # Delete ImportedFiles from previous versions + ImportedFile.objects.filter(project=version.project, + version=version + ).exclude(commit=commit).delete() + # Purge Cache + cdn_ids = getattr(settings, 'CDN_IDS', None) + if cdn_ids: + if version.project.slug in cdn_ids: + changed_files = [resolve_path( + version.project, filename=fname, version_slug=version.slug, + ) for fname in changed_files] + purge(cdn_ids[version.project.slug], changed_files) @app.task(queue='web') @@ -82,3 +91,88 @@ def fileify(version_pk, commit): msg='No ImportedFile files')) +def webhook_notification(version, build, hook_url): + """ + Send webhook notification for project webhook. + + :param version: Version instance to send hook for + :param build: Build instance that failed + :param hook_url: Hook URL to send to + """ + project = version.project + + data = json.dumps({ + 'name': project.name, + 'slug': project.slug, + 'build': { + 'id': build.id, + 'success': build.success, + 'date': build.date.strftime('%Y-%m-%d %H:%M:%S'), + } + }) + log.debug(LOG_TEMPLATE + .format(project=project.slug, version='', + msg='sending notification to: %s' % hook_url)) + try: + requests.post(hook_url, data=data) + except Exception: + log.exception('Failed to POST on webhook url: url=%s', hook_url) + + +@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) + + for hook in version.project.webhook_notifications.all(): + webhook_notification(version, build, hook.url) + for email in version.project.emailhook_notifications.all().values_list('email', flat=True): + email_notification(version, build, email) + + +def email_notification(version, build, email): + """ + Send email notifications for build failure. + + :param version: :py:class:`Version` instance that failed + :param build: :py:class:`Build` instance that failed + :param email: Email recipient address + """ + log.debug(LOG_TEMPLATE.format(project=version.project.slug, version=version.slug, + msg='sending email to: %s' % email)) + + # We send only what we need from the Django model objects here to avoid + # serialization problems in the ``readthedocs.core.tasks.send_email_task`` + context = { + 'version': { + 'verbose_name': version.verbose_name, + }, + 'project': { + 'name': version.project.name, + }, + 'build': { + 'pk': build.pk, + 'error': build.error, + }, + 'build_url': 'https://{0}{1}'.format( + getattr(settings, 'PRODUCTION_DOMAIN', 'readthedocs.org'), + build.get_absolute_url(), + ), + 'unsub_url': 'https://{0}{1}'.format( + getattr(settings, 'PRODUCTION_DOMAIN', 'readthedocs.org'), + reverse('projects_notifications', args=[version.project.slug]), + ), + } + + if build.commit: + title = _('Failed: {project[name]} ({commit})').format(commit=build.commit[:8], **context) + else: + title = _('Failed: {project[name]} ({version[verbose_name]})').format(**context) + + send_email( + email, + title, + template='projects/email/build_failed.txt', + template_html='projects/email/build_failed.html', + context=context, + ) \ No newline at end of file diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 1cd3ec7692b..9eb2bcd517c 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -39,7 +39,7 @@ from readthedocs.builds.models import Build, Version, APIVersion from readthedocs.builds.signals import build_complete from readthedocs.builds.syncers import Syncer -from readthedocs.builds.tasks import fileify +from readthedocs.builds.tasks import fileify, send_notifications, email_notification from readthedocs.cdn.purge import purge from readthedocs.core.resolver import resolve_path from readthedocs.core.symlink import PublicSymlink, PrivateSymlink @@ -905,93 +905,6 @@ def _manage_imported_files(version, path, commit): purge(cdn_ids[version.project.slug], changed_files) -@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) - - for hook in version.project.webhook_notifications.all(): - webhook_notification(version, build, hook.url) - for email in version.project.emailhook_notifications.all().values_list('email', flat=True): - email_notification(version, build, email) - - -def email_notification(version, build, email): - """ - Send email notifications for build failure. - - :param version: :py:class:`Version` instance that failed - :param build: :py:class:`Build` instance that failed - :param email: Email recipient address - """ - log.debug(LOG_TEMPLATE.format(project=version.project.slug, version=version.slug, - msg='sending email to: %s' % email)) - - # We send only what we need from the Django model objects here to avoid - # serialization problems in the ``readthedocs.core.tasks.send_email_task`` - context = { - 'version': { - 'verbose_name': version.verbose_name, - }, - 'project': { - 'name': version.project.name, - }, - 'build': { - 'pk': build.pk, - 'error': build.error, - }, - 'build_url': 'https://{0}{1}'.format( - getattr(settings, 'PRODUCTION_DOMAIN', 'readthedocs.org'), - build.get_absolute_url(), - ), - 'unsub_url': 'https://{0}{1}'.format( - getattr(settings, 'PRODUCTION_DOMAIN', 'readthedocs.org'), - reverse('projects_notifications', args=[version.project.slug]), - ), - } - - if build.commit: - title = _('Failed: {project[name]} ({commit})').format(commit=build.commit[:8], **context) - else: - title = _('Failed: {project[name]} ({version[verbose_name]})').format(**context) - - send_email( - email, - title, - template='projects/email/build_failed.txt', - template_html='projects/email/build_failed.html', - context=context, - ) - - -def webhook_notification(version, build, hook_url): - """ - Send webhook notification for project webhook. - - :param version: Version instance to send hook for - :param build: Build instance that failed - :param hook_url: Hook URL to send to - """ - project = version.project - - data = json.dumps({ - 'name': project.name, - 'slug': project.slug, - 'build': { - 'id': build.id, - 'success': build.success, - 'date': build.date.strftime('%Y-%m-%d %H:%M:%S'), - } - }) - log.debug(LOG_TEMPLATE - .format(project=project.slug, version='', - msg='sending notification to: %s' % hook_url)) - try: - requests.post(hook_url, data=data) - except Exception: - log.exception('Failed to POST on webhook url: url=%s', hook_url) - - @app.task(queue='web') def update_static_metadata(project_pk, path=None): """ From d44bdedb9adce04b8c01069d8fa4819f154fd846 Mon Sep 17 00:00:00 2001 From: shubham76 Date: Sun, 15 Apr 2018 16:42:54 +0530 Subject: [PATCH 3/8] moved the last four functions --- readthedocs/builds/tasks.py | 110 +++++++++++++++++- readthedocs/core/views/__init__.py | 2 +- readthedocs/projects/admin.py | 2 +- readthedocs/projects/tasks.py | 99 ---------------- readthedocs/projects/views/private.py | 7 +- .../tests/projects/test_admin_actions.py | 2 +- readthedocs/rtd_tests/tests/test_celery.py | 7 +- readthedocs/rtd_tests/tests/test_project.py | 2 +- .../rtd_tests/tests/test_project_views.py | 3 +- 9 files changed, 123 insertions(+), 111 deletions(-) diff --git a/readthedocs/builds/tasks.py b/readthedocs/builds/tasks.py index 4d563d4783f..f182828797a 100644 --- a/readthedocs/builds/tasks.py +++ b/readthedocs/builds/tasks.py @@ -1,15 +1,24 @@ from __future__ import absolute_import +import datetime import logging import json +import shutil import requests from django.conf import settings +from django.db.models import Q from .constants import LOG_TEMPLATE +from readthedocs.builds.constants import (LATEST, + BUILD_STATE_CLONING, + BUILD_STATE_INSTALLING, + BUILD_STATE_BUILDING, + BUILD_STATE_FINISHED) from django.core.urlresolvers import reverse from django.utils.translation import ugettext_lazy as _ +from readthedocs.doc_builder.constants import DOCKER_LIMITS from readthedocs.core.utils import send_email, broadcast from readthedocs.builds.models import Build, Version, APIVersion from readthedocs.worker import app @@ -175,4 +184,103 @@ def email_notification(version, build, email): template='projects/email/build_failed.txt', template_html='projects/email/build_failed.html', context=context, - ) \ No newline at end of file + ) + + +# Random Tasks +@app.task() +def remove_dir(path): + """ + Remove a directory on the build/celery server. + + This is mainly a wrapper around shutil.rmtree so that app servers can kill + things on the build server. + """ + log.info("Removing %s", path) + shutil.rmtree(path, ignore_errors=True) + + +@app.task() +def clear_artifacts(version_pk): + """Remove artifacts from the web servers.""" + version = Version.objects.get(pk=version_pk) + clear_pdf_artifacts(version) + clear_epub_artifacts(version) + clear_htmlzip_artifacts(version) + clear_html_artifacts(version) + + +@app.task() +def clear_pdf_artifacts(version): + if isinstance(version, int): + version = Version.objects.get(pk=version) + remove_dir(version.project.get_production_media_path( + type_='pdf', version_slug=version.slug)) + + +@app.task() +def clear_epub_artifacts(version): + if isinstance(version, int): + version = Version.objects.get(pk=version) + remove_dir(version.project.get_production_media_path( + type_='epub', version_slug=version.slug)) + + +@app.task() +def clear_htmlzip_artifacts(version): + if isinstance(version, int): + version = Version.objects.get(pk=version) + remove_dir(version.project.get_production_media_path( + type_='htmlzip', version_slug=version.slug)) + + +@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)) + + +@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/core/views/__init__.py b/readthedocs/core/views/__init__.py index aa8f2434342..043bbd1db03 100644 --- a/readthedocs/core/views/__init__.py +++ b/readthedocs/core/views/__init__.py @@ -22,7 +22,7 @@ from readthedocs.core.utils import broadcast from readthedocs.projects import constants from readthedocs.projects.models import Project, ImportedFile -from readthedocs.projects.tasks import remove_dir +from readthedocs.builds.tasks import remove_dir from readthedocs.redirects.utils import get_redirect_response log = logging.getLogger(__name__) diff --git a/readthedocs/projects/admin.py b/readthedocs/projects/admin.py index fac08e3b087..c7ec923e4a5 100644 --- a/readthedocs/projects/admin.py +++ b/readthedocs/projects/admin.py @@ -17,7 +17,7 @@ from .models import (Project, ImportedFile, Feature, ProjectRelationship, EmailHook, WebHook, Domain) from .notifications import ResourceUsageNotification -from .tasks import remove_dir +from readthedocs.builds.tasks import remove_dir class ProjectSendNotificationView(SendNotificationView): diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 9eb2bcd517c..6b492d4ca03 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -953,60 +953,6 @@ def update_static_metadata(project_pk, path=None): )) -# Random Tasks -@app.task() -def remove_dir(path): - """ - Remove a directory on the build/celery server. - - This is mainly a wrapper around shutil.rmtree so that app servers can kill - things on the build server. - """ - log.info("Removing %s", path) - shutil.rmtree(path, ignore_errors=True) - - -@app.task() -def clear_artifacts(version_pk): - """Remove artifacts from the web servers.""" - version = Version.objects.get(pk=version_pk) - clear_pdf_artifacts(version) - clear_epub_artifacts(version) - clear_htmlzip_artifacts(version) - clear_html_artifacts(version) - - -@app.task() -def clear_pdf_artifacts(version): - if isinstance(version, int): - version = Version.objects.get(pk=version) - remove_dir(version.project.get_production_media_path( - type_='pdf', version_slug=version.slug)) - - -@app.task() -def clear_epub_artifacts(version): - if isinstance(version, int): - version = Version.objects.get(pk=version) - remove_dir(version.project.get_production_media_path( - type_='epub', version_slug=version.slug)) - - -@app.task() -def clear_htmlzip_artifacts(version): - if isinstance(version, int): - version = Version.objects.get(pk=version) - remove_dir(version.project.get_production_media_path( - type_='htmlzip', version_slug=version.slug)) - - -@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)) - - @app.task(queue='web') def sync_callback(_, version_pk, commit, *args, **kwargs): """ @@ -1016,48 +962,3 @@ 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/projects/views/private.py b/readthedocs/projects/views/private.py index 5ae4d68602e..8c4e4599851 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -31,6 +31,7 @@ from readthedocs.oauth.services import registry from readthedocs.oauth.utils import attach_webhook, update_webhook from readthedocs.projects import tasks +from readthedocs.builds.tasks import remove_dir, clear_artifacts from readthedocs.projects.forms import ( DomainForm, EmailHookForm, IntegrationForm, ProjectAdvancedForm, ProjectAdvertisingForm, ProjectBasicsForm, ProjectExtraForm, @@ -175,7 +176,7 @@ def project_version_detail(request, project_slug, version_slug): if 'active' in form.changed_data and version.active is False: log.info('Removing files for version %s', version.slug) broadcast( - type='app', task=tasks.clear_artifacts, args=[version.pk]) + type='app', task=clear_artifacts, args=[version.pk]) version.built = False version.save() url = reverse('project_version_list', args=[project.slug]) @@ -198,7 +199,7 @@ def project_delete(request, project_slug): Project.objects.for_admin_user(request.user), slug=project_slug) if request.method == 'POST': - broadcast(type='app', task=tasks.remove_dir, args=[project.doc_path]) + broadcast(type='app', task=remove_dir, args=[project.doc_path]) project.delete() messages.success(request, _('Project deleted')) project_dashboard = reverse('projects_dashboard') @@ -669,7 +670,7 @@ def project_version_delete_html(request, project_slug, version_slug): if not version.active: version.built = False version.save() - broadcast(type='app', task=tasks.clear_artifacts, args=[version.pk]) + broadcast(type='app', task=clear_artifacts, args=[version.pk]) else: return HttpResponseBadRequest( "Can't delete HTML for an active version.") diff --git a/readthedocs/rtd_tests/tests/projects/test_admin_actions.py b/readthedocs/rtd_tests/tests/projects/test_admin_actions.py index 7bda4501bf7..3f5fdaba673 100644 --- a/readthedocs/rtd_tests/tests/projects/test_admin_actions.py +++ b/readthedocs/rtd_tests/tests/projects/test_admin_actions.py @@ -60,7 +60,7 @@ def test_project_ban_multiple_owners(self): @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 + from readthedocs.builds.tasks import remove_dir action_data = { ACTION_CHECKBOX_NAME: [self.project.pk], 'action': 'delete_selected', diff --git a/readthedocs/rtd_tests/tests/test_celery.py b/readthedocs/rtd_tests/tests/test_celery.py index e9124baf104..75b46b95a07 100644 --- a/readthedocs/rtd_tests/tests/test_celery.py +++ b/readthedocs/rtd_tests/tests/test_celery.py @@ -12,6 +12,7 @@ from readthedocs.builds.models import Build from readthedocs.projects.models import Project from readthedocs.projects import tasks +from readthedocs.builds.tasks import remove_dir, clear_artifacts from readthedocs.rtd_tests.utils import make_test_git from readthedocs.rtd_tests.base import RTDTestCase @@ -44,7 +45,7 @@ def tearDown(self): def test_remove_dir(self): directory = mkdtemp() self.assertTrue(exists(directory)) - result = tasks.remove_dir.delay(directory) + result = remove_dir.delay(directory) self.assertTrue(result.successful()) self.assertFalse(exists(directory)) @@ -53,14 +54,14 @@ def test_clear_artifacts(self): directory = self.project.get_production_media_path(type_='pdf', version_slug=version.slug) os.makedirs(directory) self.assertTrue(exists(directory)) - result = tasks.clear_artifacts.delay(version_pk=version.pk) + result = clear_artifacts.delay(version_pk=version.pk) self.assertTrue(result.successful()) self.assertFalse(exists(directory)) directory = version.project.rtd_build_path(version=version.slug) os.makedirs(directory) self.assertTrue(exists(directory)) - result = tasks.clear_artifacts.delay(version_pk=version.pk) + result = clear_artifacts.delay(version_pk=version.pk) self.assertTrue(result.successful()) self.assertFalse(exists(directory)) diff --git a/readthedocs/rtd_tests/tests/test_project.py b/readthedocs/rtd_tests/tests/test_project.py index 4e0ca1badb4..857458ac146 100644 --- a/readthedocs/rtd_tests/tests/test_project.py +++ b/readthedocs/rtd_tests/tests/test_project.py @@ -16,7 +16,7 @@ from readthedocs.builds.models import Build from readthedocs.projects.exceptions import ProjectConfigurationError from readthedocs.projects.models import Project -from readthedocs.projects.tasks import finish_inactive_builds +from readthedocs.builds.tasks import finish_inactive_builds from readthedocs.rtd_tests.mocks.paths import fake_paths_by_regex diff --git a/readthedocs/rtd_tests/tests/test_project_views.py b/readthedocs/rtd_tests/tests/test_project_views.py index b52dbe40f75..13ed6fb4446 100644 --- a/readthedocs/rtd_tests/tests/test_project_views.py +++ b/readthedocs/rtd_tests/tests/test_project_views.py @@ -23,6 +23,7 @@ from readthedocs.projects.views.private import ImportWizardView from readthedocs.projects.views.mixins import ProjectRelationMixin from readthedocs.projects import tasks +from readthedocs.builds.tasks import remove_dir @patch('readthedocs.projects.views.private.trigger_build', lambda x, basic: None) @@ -379,7 +380,7 @@ def test_delete_project(self): self.assertFalse(Project.objects.filter(slug='pip').exists()) broadcast.assert_called_with( type='app', - task=tasks.remove_dir, + task=remove_dir, args=[project.doc_path]) def test_subproject_create(self): From b4febee418efd2c88f26ff6b0f9c7f0efc24bb95 Mon Sep 17 00:00:00 2001 From: shubham76 Date: Mon, 16 Apr 2018 00:08:36 +0530 Subject: [PATCH 4/8] fixed linting errors --- readthedocs/builds/tasks.py | 7 ++++++- readthedocs/projects/tasks.py | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/readthedocs/builds/tasks.py b/readthedocs/builds/tasks.py index f182828797a..4c74bc4bd9f 100644 --- a/readthedocs/builds/tasks.py +++ b/readthedocs/builds/tasks.py @@ -4,19 +4,24 @@ import datetime import logging import json +import os import shutil +import hashlib import requests from django.conf import settings from django.db.models import Q from .constants import LOG_TEMPLATE +from readthedocs.projects.models import ImportedFile, Project, Domain from readthedocs.builds.constants import (LATEST, BUILD_STATE_CLONING, BUILD_STATE_INSTALLING, BUILD_STATE_BUILDING, BUILD_STATE_FINISHED) +from readthedocs.cdn.purge import purge from django.core.urlresolvers import reverse +from readthedocs.core.resolver import resolve_path from django.utils.translation import ugettext_lazy as _ from readthedocs.doc_builder.constants import DOCKER_LIMITS from readthedocs.core.utils import send_email, broadcast @@ -25,7 +30,7 @@ log = logging.getLogger(__name__) -#copy of this function exists in projects/tasks.py + def _manage_imported_files(version, path, commit): """ Update imported files for version. diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 6b492d4ca03..dd74c12c88f 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -39,7 +39,7 @@ from readthedocs.builds.models import Build, Version, APIVersion from readthedocs.builds.signals import build_complete from readthedocs.builds.syncers import Syncer -from readthedocs.builds.tasks import fileify, send_notifications, email_notification +from readthedocs.builds.tasks import fileify, send_notifications, email_notification, clear_pdf_artifacts, clear_epub_artifacts from readthedocs.cdn.purge import purge from readthedocs.core.resolver import resolve_path from readthedocs.core.symlink import PublicSymlink, PrivateSymlink From 53a1bac53e5cfa12c49b15b3d19b5de5696e3e83 Mon Sep 17 00:00:00 2001 From: shubham76 Date: Sun, 15 Apr 2018 19:02:08 +0530 Subject: [PATCH 5/8] Updated builds/tasks.py --- readthedocs/projects/tasks.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index dd74c12c88f..2445eb80084 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -39,7 +39,8 @@ from readthedocs.builds.models import Build, Version, APIVersion from readthedocs.builds.signals import build_complete from readthedocs.builds.syncers import Syncer -from readthedocs.builds.tasks import fileify, send_notifications, email_notification, clear_pdf_artifacts, clear_epub_artifacts +from readthedocs.builds.tasks import fileify, send_notifications, email_notification +from readthedocs.builds.tasks import clear_pdf_artifacts, clear_epub_artifacts from readthedocs.cdn.purge import purge from readthedocs.core.resolver import resolve_path from readthedocs.core.symlink import PublicSymlink, PrivateSymlink From 4daf6770243d101c5d71fa9d8df5555882517e71 Mon Sep 17 00:00:00 2001 From: shubham76 Date: Tue, 17 Apr 2018 10:06:10 +0530 Subject: [PATCH 6/8] removed duplicated function --- readthedocs/projects/tasks.py | 45 ------------------- .../rtd_tests/tests/test_imported_file.py | 2 +- 2 files changed, 1 insertion(+), 46 deletions(-) diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 2445eb80084..d41a595b57c 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -861,51 +861,6 @@ def symlink_subproject(project_pk): sym.symlink_subprojects() -def _manage_imported_files(version, path, commit): - """ - Update imported files for version. - - :param version: Version instance - :param path: Path to search - :param commit: Commit that updated path - """ - changed_files = set() - for root, __, filenames in os.walk(path): - for filename in filenames: - dirpath = os.path.join(root.replace(path, '').lstrip('/'), - filename.lstrip('/')) - full_path = os.path.join(root, filename) - md5 = hashlib.md5(open(full_path, 'rb').read()).hexdigest() - try: - obj, __ = ImportedFile.objects.get_or_create( - project=version.project, - version=version, - path=dirpath, - name=filename, - ) - except ImportedFile.MultipleObjectsReturned: - log.warning('Error creating ImportedFile') - continue - if obj.md5 != md5: - obj.md5 = md5 - changed_files.add(dirpath) - if obj.commit != commit: - obj.commit = commit - obj.save() - # Delete ImportedFiles from previous versions - ImportedFile.objects.filter(project=version.project, - version=version - ).exclude(commit=commit).delete() - # Purge Cache - cdn_ids = getattr(settings, 'CDN_IDS', None) - if cdn_ids: - if version.project.slug in cdn_ids: - changed_files = [resolve_path( - version.project, filename=fname, version_slug=version.slug, - ) for fname in changed_files] - purge(cdn_ids[version.project.slug], changed_files) - - @app.task(queue='web') def update_static_metadata(project_pk, path=None): """ diff --git a/readthedocs/rtd_tests/tests/test_imported_file.py b/readthedocs/rtd_tests/tests/test_imported_file.py index 81f5e2a684b..c136ad3255f 100644 --- a/readthedocs/rtd_tests/tests/test_imported_file.py +++ b/readthedocs/rtd_tests/tests/test_imported_file.py @@ -2,7 +2,7 @@ import os from django.test import TestCase -from readthedocs.projects.tasks import _manage_imported_files +from readthedocs.builds.tasks import _manage_imported_files from readthedocs.projects.models import Project, ImportedFile base_dir = os.path.dirname(os.path.dirname(__file__)) From 0e44d7e1fc5a038cceea46e79acafd51f85444f2 Mon Sep 17 00:00:00 2001 From: shubham76 Date: Wed, 18 Apr 2018 19:00:56 +0530 Subject: [PATCH 7/8] fixed in other places according refactoring --- docs/locale/fa/LC_MESSAGES/api/projects.po | 4 ++-- docs/locale/pt_BR/LC_MESSAGES/api/projects.po | 4 ++-- readthedocs/rtd_tests/tests/test_build_notifications.py | 2 +- readthedocs/settings/base.py | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/locale/fa/LC_MESSAGES/api/projects.po b/docs/locale/fa/LC_MESSAGES/api/projects.po index f69db138e67..3b461dd4e23 100644 --- a/docs/locale/fa/LC_MESSAGES/api/projects.po +++ b/docs/locale/fa/LC_MESSAGES/api/projects.po @@ -279,12 +279,12 @@ msgid "" msgstr "" # 6a2b786705274c008323db14a41b81c7 -#: ../../../readthedocs/projects/tasks.pydocstring of projects.tasks.fileify:1 +#: ../../../readthedocs/projects/tasks.pydocstring of projects.builds.fileify:1 msgid "Create ImportedFile objects for all of a version's files." msgstr "" # 78e3cfd69fe14addb9dd6c3ccfe9c8cd -#: ../../../readthedocs/projects/tasks.pydocstring of projects.tasks.fileify:3 +#: ../../../readthedocs/projects/tasks.pydocstring of projects.builds.fileify:3 msgid "" "This is a prereq for indexing the docs for search. It also causes celery-" "haystack to kick off an index of the file." diff --git a/docs/locale/pt_BR/LC_MESSAGES/api/projects.po b/docs/locale/pt_BR/LC_MESSAGES/api/projects.po index 862e2ab7f0a..80e373bdefc 100644 --- a/docs/locale/pt_BR/LC_MESSAGES/api/projects.po +++ b/docs/locale/pt_BR/LC_MESSAGES/api/projects.po @@ -279,12 +279,12 @@ msgid "" msgstr "" # 6a2b786705274c008323db14a41b81c7 -#: ../../../readthedocs/projects/tasks.pydocstring of projects.tasks.fileify:1 +#: ../../../readthedocs/projects/tasks.pydocstring of projects.builds.fileify:1 msgid "Create ImportedFile objects for all of a version's files." msgstr "" # 78e3cfd69fe14addb9dd6c3ccfe9c8cd -#: ../../../readthedocs/projects/tasks.pydocstring of projects.tasks.fileify:3 +#: ../../../readthedocs/projects/tasks.pydocstring of projects.builds.fileify:3 msgid "" "This is a prereq for indexing the docs for search. It also causes celery-" "haystack to kick off an index of the file." diff --git a/readthedocs/rtd_tests/tests/test_build_notifications.py b/readthedocs/rtd_tests/tests/test_build_notifications.py index 126badb0497..bd9b5c95a5f 100644 --- a/readthedocs/rtd_tests/tests/test_build_notifications.py +++ b/readthedocs/rtd_tests/tests/test_build_notifications.py @@ -11,7 +11,7 @@ from readthedocs.builds.models import Build, Version from readthedocs.projects.models import Project, EmailHook, WebHook -from readthedocs.projects.tasks import send_notifications +from readthedocs.builds.tasks import send_notifications class BuildNotificationsTests(TestCase): diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index 8057901534f..7b7a7bf4f6d 100644 --- a/readthedocs/settings/base.py +++ b/readthedocs/settings/base.py @@ -254,7 +254,7 @@ def USE_PROMOS(self): # noqa 'options': {'queue': 'web'}, }, 'quarter-finish-inactive-builds': { - 'task': 'readthedocs.projects.tasks.finish_inactive_builds', + 'task': 'readthedocs.projects.builds.finish_inactive_builds', 'schedule': crontab(minute='*/15'), 'options': {'queue': 'web'}, }, From caec686d50f0f4cfa2ad5325f56e2e22fe4a0430 Mon Sep 17 00:00:00 2001 From: shubham76 Date: Sat, 21 Apr 2018 14:19:43 +0530 Subject: [PATCH 8/8] Fixed liinting errors --- readthedocs/bookmarks/views.py | 56 +++++++++------------------------- 1 file changed, 14 insertions(+), 42 deletions(-) diff --git a/readthedocs/bookmarks/views.py b/readthedocs/bookmarks/views.py index 9a790c3ffbe..b1861d44e86 100644 --- a/readthedocs/bookmarks/views.py +++ b/readthedocs/bookmarks/views.py @@ -9,8 +9,7 @@ from django.contrib.auth.decorators import login_required from django.core.exceptions import ObjectDoesNotExist from django.core.urlresolvers import reverse -from django.http import ( - HttpResponse, HttpResponseBadRequest, HttpResponseRedirect) +from django.http import HttpResponseRedirect, JsonResponse from django.shortcuts import get_object_or_404, render from django.utils.decorators import method_decorator from django.views.decorators.csrf import csrf_exempt @@ -32,9 +31,8 @@ def dispatch(self, request, *args, **kwargs): self).dispatch(request, *args, **kwargs) def get(self, request): - return HttpResponse( - content=json.dumps({'error': 'You must POST!'}), - content_type='application/json', + return JsonResponse( + {'error': 'You must POST!'}, status=405, ) @@ -52,9 +50,7 @@ def post(self, request, *args, **kwargs): version = post_json['version'] page = post_json['page'] except KeyError: - return HttpResponseBadRequest( - content=json.dumps({'error': 'Invalid parameters'}), - ) + return JsonResponse({'error': 'Invalid parameters'}, status=400) try: Bookmark.objects.get( project__slug=project, @@ -62,17 +58,9 @@ def post(self, request, *args, **kwargs): page=page, ) except ObjectDoesNotExist: - return HttpResponse( - content=json.dumps({'exists': False}), - status=404, - content_type='application/json', - ) + return JsonResponse({'exists': False}, status=404) - return HttpResponse( - content=json.dumps({'exists': True}), - status=200, - content_type='application/json', - ) + return JsonResponse({'exists': True}) class BookmarkListView(ListView): @@ -99,11 +87,7 @@ def dispatch(self, request, *args, **kwargs): return super(BookmarkAddView, self).dispatch(request, *args, **kwargs) def get(self, request): - return HttpResponse( - content=json.dumps({'error': 'You must POST!'}), - content_type='application/json', - status=405, - ) + return JsonResponse({'error': 'You must POST!'}, status=405) def post(self, request, *args, **kwargs): """ @@ -118,17 +102,15 @@ def post(self, request, *args, **kwargs): page_slug = post_json['page'] url = post_json['url'] except KeyError: - return HttpResponseBadRequest( - content=json.dumps({'error': 'Invalid parameters'}), - ) + return JsonResponse({'error': 'Invalid parameters'}, status=400) try: project = Project.objects.get(slug=project_slug) version = project.versions.get(slug=version_slug) except ObjectDoesNotExist: - return HttpResponseBadRequest( - content=json.dumps( - {'error': 'Project or Version does not exist'}), + return JsonResponse( + {'error': 'Project or Version does not exist'}, + status=400, ) Bookmark.objects.get_or_create( @@ -138,11 +120,7 @@ def post(self, request, *args, **kwargs): version=version, page=page_slug, ) - return HttpResponse( - json.dumps({'added': True}), - status=201, - content_type='application/json', - ) + return JsonResponse({'added': True}, status=201) class BookmarkRemoveView(View): @@ -179,9 +157,7 @@ def post(self, request, *args, **kwargs): url = post_json['url'] page = post_json['page'] except KeyError: - return HttpResponseBadRequest( - json.dumps({'error': 'Invalid parameters'}), - ) + return JsonResponse({'error': 'Invalid parameters'}, status=400) bookmark = get_object_or_404( Bookmark, @@ -193,8 +169,4 @@ def post(self, request, *args, **kwargs): ) bookmark.delete() - return HttpResponse( - json.dumps({'removed': True}), - status=200, - content_type='application/json', - ) + return JsonResponse({'removed': True})