diff --git a/readthedocs/api/v2/views/task_views.py b/readthedocs/api/v2/views/task_views.py index b5271271641..7d893963973 100644 --- a/readthedocs/api/v2/views/task_views.py +++ b/readthedocs/api/v2/views/task_views.py @@ -1,61 +1,45 @@ """Endpoints relating to task/job status, etc.""" import structlog - +from django.core.cache import cache from django.urls import reverse -from redis import ConnectionError from rest_framework import decorators, permissions from rest_framework.renderers import JSONRenderer from rest_framework.response import Response -from readthedocs.core.utils.tasks import TaskNoPermission, get_public_task_data from readthedocs.oauth import tasks - log = structlog.get_logger(__name__) -SUCCESS_STATES = ('SUCCESS',) -FAILURE_STATES = ( - 'FAILURE', - 'REVOKED', -) -FINISHED_STATES = SUCCESS_STATES + FAILURE_STATES -STARTED_STATES = ('RECEIVED', 'STARTED', 'RETRY') + FINISHED_STATES - - -def get_status_data(task_name, state, data, error=None): - data = { - 'name': task_name, - 'data': data, - 'started': state in STARTED_STATES, - 'finished': state in FINISHED_STATES, - # When an exception is raised inside the task, we keep this as SUCCESS - # and add the exception message into the 'error' key - 'success': state in SUCCESS_STATES and error is None, - } - if error is not None: - data['error'] = error - return data - @decorators.api_view(['GET']) @decorators.permission_classes((permissions.AllowAny,)) @decorators.renderer_classes((JSONRenderer,)) def job_status(request, task_id): - try: - task_name, state, public_data, error = get_public_task_data( - request, - task_id, - ) - except (TaskNoPermission, ConnectionError): - return Response(get_status_data('unknown', 'PENDING', {}),) - return Response(get_status_data(task_name, state, public_data, error),) + """Retrieve Celery task function state from frontend.""" + # HACK: always poll up to N times and after that return the sync has + # finished. This is a way to avoid re-enabling Celery result backend for now. + # TODO remove this API and RemoteRepo sync UI when we have better auto syncing + poll_n = cache.get(task_id, 0) + poll_n += 1 + cache.set(task_id, poll_n, 5 * 60) + finished = poll_n == 5 + + data = { + "name": "sync_remote_repositories", + "data": {}, + "started": True, + "finished": finished, + "success": finished, + } + return Response(data) @decorators.api_view(['POST']) @decorators.permission_classes((permissions.IsAuthenticated,)) @decorators.renderer_classes((JSONRenderer,)) def sync_remote_repositories(request): + """Trigger a re-sync of remote repositories for the user.""" result = tasks.sync_remote_repositories.delay(user_id=request.user.id,) task_id = result.task_id return Response({ diff --git a/readthedocs/core/utils/tasks/__init__.py b/readthedocs/core/utils/tasks/__init__.py index 1bd648a0796..8db5b6e3420 100644 --- a/readthedocs/core/utils/tasks/__init__.py +++ b/readthedocs/core/utils/tasks/__init__.py @@ -6,6 +6,4 @@ ) from .public import PublicTask # noqa from .public import TaskNoPermission # noqa -from .public import get_public_task_data # noqa from .retrieve import TaskNotFound # noqa -from .retrieve import get_task_data # noqa diff --git a/readthedocs/core/utils/tasks/public.py b/readthedocs/core/utils/tasks/public.py index 621e2798623..b19cef77002 100644 --- a/readthedocs/core/utils/tasks/public.py +++ b/readthedocs/core/utils/tasks/public.py @@ -1,22 +1,19 @@ -# -*- coding: utf-8 -*- - """Celery tasks with publicly viewable status.""" from celery import Task, states from django.conf import settings -from .retrieve import TaskNotFound, get_task_data - - __all__ = ( 'PublicTask', 'TaskNoPermission', - 'get_public_task_data', ) STATUS_UPDATES_ENABLED = not settings.CELERY_ALWAYS_EAGER +# pylint: disable=abstract-method +# pylint: disable=broad-except +# pylint: disable=invalid-name class PublicTask(Task): """ @@ -121,35 +118,3 @@ def __init__(self, task_id, *args, **kwargs): id=task_id, ) super().__init__(message, *args, **kwargs) - - -def get_public_task_data(request, task_id): - """ - Return task details as tuple. - - Will raise `TaskNoPermission` if `request` has no permission to access info - of the task with id `task_id`. This is also the case of no task with the - given id exists. - - :returns: (task name, task state, public data, error message) - :rtype: (str, str, dict, str) - """ - try: - task, state, info = get_task_data(task_id) - except TaskNotFound: - # No task info has been found act like we don't have permission to see - # the results. - raise TaskNoPermission(task_id) - - if not hasattr(task, 'check_permission'): - raise TaskNoPermission(task_id) - - context = info.get('context', {}) - if not task.check_permission(request, state, context): - raise TaskNoPermission(task_id) - return ( - task.name, - state, - info.get('public_data', {}), - info.get('error', None), - ) diff --git a/readthedocs/core/utils/tasks/retrieve.py b/readthedocs/core/utils/tasks/retrieve.py index 9281ad8a1af..8257ef9e386 100644 --- a/readthedocs/core/utils/tasks/retrieve.py +++ b/readthedocs/core/utils/tasks/retrieve.py @@ -1,12 +1,8 @@ -# -*- coding: utf-8 -*- """Utilities for retrieving task data.""" -from celery import states -from celery.result import AsyncResult - -__all__ = ('TaskNotFound', 'get_task_data') +__all__ = ("TaskNotFound",) class TaskNotFound(Exception): @@ -14,24 +10,3 @@ class TaskNotFound(Exception): def __init__(self, task_id, *args, **kwargs): message = 'No public task found with id {id}'.format(id=task_id) super().__init__(message, *args, **kwargs) - - -def get_task_data(task_id): - """ - Will raise `TaskNotFound` if the task is in state ``PENDING`` or the task. - - 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 == states.PENDING: - raise TaskNotFound(task_id) - if 'task_name' not in info: - raise TaskNotFound(task_id) - try: - task = app.tasks[info['task_name']] - except KeyError: - raise TaskNotFound(task_id) - return task, state, info diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index 29974c006f8..0063f76ab40 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -38,7 +38,6 @@ GitHubWebhookView, GitLabWebhookView, ) -from readthedocs.api.v2.views.task_views import get_status_data from readthedocs.builds.constants import ( BUILD_STATE_CLONING, BUILD_STATE_FINISHED, @@ -2545,24 +2544,3 @@ def test_modify_version(self): self.assertEqual(resp.data['has_pdf'], True) self.assertEqual(resp.data['has_epub'], False) self.assertEqual(resp.data['has_htmlzip'], False) - - -class TaskViewsTests(TestCase): - - def test_get_status_data(self): - data = get_status_data( - 'public_task_exception', - 'SUCCESS', - {'data': 'public'}, - 'Something bad happened', - ) - self.assertEqual( - data, { - 'name': 'public_task_exception', - 'data': {'data': 'public'}, - 'started': True, - 'finished': True, - 'success': False, - 'error': 'Something bad happened', - }, - ) diff --git a/readthedocs/rtd_tests/tests/test_privacy_urls.py b/readthedocs/rtd_tests/tests/test_privacy_urls.py index c351e305cee..a7ab1d43dbb 100644 --- a/readthedocs/rtd_tests/tests/test_privacy_urls.py +++ b/readthedocs/rtd_tests/tests/test_privacy_urls.py @@ -15,7 +15,6 @@ RegexAutomationRule, VersionAutomationRule, ) -from readthedocs.core.utils.tasks import TaskNoPermission from readthedocs.integrations.models import HttpExchange, Integration from readthedocs.oauth.models import RemoteOrganization, RemoteRepository from readthedocs.projects.models import Domain, EnvironmentVariable, Project, WebHook @@ -452,21 +451,6 @@ def setUp(self): } -class APIUnauthAccessTest(APIMixin, TestCase): - - @mock.patch('readthedocs.api.v2.views.task_views.get_public_task_data') - def test_api_urls(self, get_public_task_data): - from readthedocs.api.v2.urls import urlpatterns - get_public_task_data.side_effect = TaskNoPermission('Nope') - self._test_url(urlpatterns) - - def login(self): - pass - - def is_admin(self): - return False - - class PublicUserProfileMixin(URLAccessMixin): def setUp(self):