Save build data using a task instead of using the API

Author: stsewd

readthedocs/api/v2/views/model_views.py

@@ -19,7 +19,6 @@
 from readthedocs.oauth.services import GitHubService, registry
 from readthedocs.projects.models import Domain, Project
 from readthedocs.storage import build_commands_storage
-from readthedocs.telemetry.models import BuildData
 
 from ..permissions import APIPermission, APIRestrictedPermission, IsOwner
 from ..serializers import (
@@ -286,17 +285,6 @@ def reset(self, request, **kwargs):
         instance.reset()
         return Response(status=status.HTTP_204_NO_CONTENT)
 
-    @decorators.action(
-        detail=True,
-        permission_classes=[permissions.IsAdminUser],
-        methods=["post"],
-    )
-    def telemetry(self, request, **kwargs):
-        """Collect telemetry data from the build."""
-        build = self.get_object()
-        BuildData.objects.collect(build, request.data)
-        return Response(status=status.HTTP_204_NO_CONTENT)
-
 
 class BuildCommandViewSet(DisableListEndpoint, UserSelectViewSet):
     parser_classes = [JSONParser, MultiPartParser]

readthedocs/projects/tasks/builds.py

@@ -47,6 +47,7 @@
 )
 from readthedocs.storage import build_media_storage
 from readthedocs.telemetry.collectors import BuildDataCollector
+from readthedocs.telemetry.tasks import save_build_data
 from readthedocs.worker import app
 
 from ..exceptions import (
@@ -543,7 +544,7 @@ def after_return(self, status, retval, task_id, args, kwargs, einfo):
         self.data.build['length'] = (timezone.now() - self.data.start_time).seconds
 
         self.update_build(BUILD_STATE_FINISHED)
-        self.upload_build_data()
+        self.save_build_data()
 
         build_complete.send(sender=Build, build=self.data.build)
 
@@ -624,18 +625,21 @@ def collect_build_data(self):
         except Exception:
             log.exception("Error while collecting build data")
 
-    def upload_build_data(self):
+    def save_build_data(self):
         """
-        Upload data collected from the build after the build has ended.
+        Save the data collected from the build after it has ended.
 
         This must be called after the build has finished updating its state,
         otherwise some attributes like ``length`` won't be available.
         """
         try:
             if self.data.build_data:
-                api_v2.build(self.data.build_pk).telemetry.post(self.data.build_data)
+                save_build_data.delay(
+                    build_id=self.data.build_pk,
+                    data=self.data.build_data,
+                )
         except Exception:
-            log.exception("Error while uploading build data")
+            log.exception("Error while saving build data")
 
     @staticmethod
     def get_project(project_pk):

readthedocs/projects/tests/test_build_tasks.py

@@ -17,6 +17,7 @@
 from readthedocs.projects.exceptions import RepositoryError
 from readthedocs.projects.models import EnvironmentVariable, Project, WebHookEvent
 from readthedocs.projects.tasks.builds import sync_repository_task, update_docs_task
+from readthedocs.telemetry.models import BuildData
 
 from .mockers import BuildEnvironmentMocker
 
@@ -258,6 +259,8 @@ def test_successful_build(
             }
         )
 
+        assert not BuildData.objects.all().exists()
+
         self._trigger_update_docs_task()
 
         # It has to be called twice, ``before_start`` and ``after_return``
@@ -393,8 +396,7 @@ def test_successful_build(
             "error": "",
         }
 
-        request = self.requests_mock.request_history[10]
-        assert request.path == "/api/v2/build/1/telemetry/"
+        assert BuildData.objects.all().exists()
 
         self.mocker.mocks["build_media_storage"].sync_directory.assert_has_calls(
             [
@@ -410,7 +412,6 @@ def test_successful_build(
 
     @mock.patch("readthedocs.projects.tasks.builds.build_complete")
     @mock.patch("readthedocs.projects.tasks.builds.send_external_build_status")
-    @mock.patch("readthedocs.projects.tasks.builds.UpdateDocsTask.upload_build_data")
     @mock.patch("readthedocs.projects.tasks.builds.UpdateDocsTask.execute")
     @mock.patch("readthedocs.projects.tasks.builds.UpdateDocsTask.send_notifications")
     @mock.patch("readthedocs.projects.tasks.builds.clean_build")
@@ -419,10 +420,11 @@ def test_failed_build(
         clean_build,
         send_notifications,
         execute,
-        upload_build_data,
         send_external_build_status,
         build_complete,
     ):
+        assert not BuildData.objects.all().exists()
+
         # Force an exception from the execution of the task. We don't really
         # care "where" it was raised: setup, build, syncing directories, etc
         execute.side_effect = Exception('Force and exception here.')
@@ -455,8 +457,8 @@ def test_failed_build(
         )
 
         # The build data is None (we are failing the build before the environment is created)
-        # and the API won't be hit, but we can test that the method was called at least.
-        upload_build_data.assert_called_once()
+        # and the task won't be run.
+        assert not BuildData.objects.all().exists()
 
         # Test we are updating the DB by calling the API with the updated build object
         api_request = self.requests_mock.request_history[

readthedocs/rtd_tests/tests/test_privacy_urls.py

@@ -18,7 +18,12 @@
 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
+from readthedocs.projects.models import (
+    Domain,
+    EnvironmentVariable,
+    Project,
+    WebHook,
+)
 from readthedocs.rtd_tests.utils import create_user
 
 
@@ -394,36 +399,35 @@ def setUp(self):
             'api_webhook_stripe': {},
         }
         self.response_data = {
-            "domain-list": {"status_code": 410},
-            "buildcommandresult-list": {"status_code": 410},
-            "build-concurrent": {"status_code": 403},
-            "build-telemetry": {"status_code": 403},
-            "build-list": {"status_code": 410},
-            "build-reset": {"status_code": 403},
-            "project-sync-versions": {"status_code": 403},
-            "project-token": {"status_code": 403},
-            "emailhook-list": {"status_code": 403},
-            "emailhook-detail": {"status_code": 403},
-            "embed": {"status_code": 400},
-            "docurl": {"status_code": 400},
-            "cname": {"status_code": 400},
-            "index_search": {"status_code": 403},
-            "api_search": {"status_code": 400},
-            "api_project_search": {"status_code": 400},
-            "api_section_search": {"status_code": 400},
-            "api_sync_remote_repositories": {"status_code": 403},
-            "api_webhook": {"status_code": 405},
-            "api_webhook_github": {"status_code": 405},
-            "api_webhook_gitlab": {"status_code": 405},
-            "api_webhook_bitbucket": {"status_code": 405},
-            "api_webhook_generic": {"status_code": 403},
-            "api_webhook_stripe": {"status_code": 405},
-            "sphinxdomain-detail": {"status_code": 404},
-            "project-list": {"status_code": 410},
-            "remoteorganization-detail": {"status_code": 404},
-            "remoterepository-detail": {"status_code": 404},
-            "remoteaccount-detail": {"status_code": 404},
-            "version-list": {"status_code": 410},
+            'domain-list': {'status_code': 410},
+            'buildcommandresult-list': {'status_code': 410},
+            'build-concurrent': {'status_code': 403},
+            'build-list': {'status_code': 410},
+            'build-reset': {'status_code': 403},
+            'project-sync-versions': {'status_code': 403},
+            'project-token': {'status_code': 403},
+            'emailhook-list': {'status_code': 403},
+            'emailhook-detail': {'status_code': 403},
+            'embed': {'status_code': 400},
+            'docurl': {'status_code': 400},
+            'cname': {'status_code': 400},
+            'index_search': {'status_code': 403},
+            'api_search': {'status_code': 400},
+            'api_project_search': {'status_code': 400},
+            'api_section_search': {'status_code': 400},
+            'api_sync_remote_repositories': {'status_code': 403},
+            'api_webhook': {'status_code': 405},
+            'api_webhook_github': {'status_code': 405},
+            'api_webhook_gitlab': {'status_code': 405},
+            'api_webhook_bitbucket': {'status_code': 405},
+            'api_webhook_generic': {'status_code': 403},
+            'api_webhook_stripe': {'status_code': 405},
+            'sphinxdomain-detail': {'status_code': 404},
+            'project-list': {'status_code': 410},
+            'remoteorganization-detail': {'status_code': 404},
+            'remoterepository-detail': {'status_code': 404},
+            'remoteaccount-detail': {'status_code': 404},
+            'version-list': {'status_code': 410},
         }
 
 

readthedocs/telemetry/apps.py

@@ -10,3 +10,6 @@
 class TelemetryConfig(AppConfig):
     default_auto_field = "django.db.models.BigAutoField"
     name = "readthedocs.telemetry"
+
+    def ready(self):
+        import readthedocs.telemetry.tasks  # noqa

readthedocs/telemetry/tasks.py

@@ -0,0 +1,10 @@
+from readthedocs.builds.models import Build
+from readthedocs.telemetry.models import BuildData
+from readthedocs.worker import app
+
+
+@app.task(queue="web")
+def save_build_data(build_id, data):
+    build = Build.objects.filter(id=build_id).first()
+    if build:
+        BuildData.objects.collect(build, data)