Store v2 reports in GCS instead of Redis (#960)

Author: Swatinem

services/task/task.py

@@ -134,6 +134,7 @@ def upload_signature(
         commitid,
         report_type=None,
         report_code=None,
+        arguments=None,
         debug=False,
         rebuild=False,
         immutable=False,
@@ -145,6 +146,7 @@ def upload_signature(
                 commitid=commitid,
                 report_type=report_type,
                 report_code=report_code,
+                arguments=arguments,
                 debug=debug,
                 rebuild=rebuild,
             ),
@@ -157,6 +159,7 @@ def upload(
         commitid,
         report_type=None,
         report_code=None,
+        arguments=None,
         countdown=0,
         debug=False,
         rebuild=False,
@@ -166,6 +169,7 @@ def upload(
             commitid,
             report_type=report_type,
             report_code=report_code,
+            arguments=arguments,
             debug=debug,
             rebuild=rebuild,
         ).apply_async(countdown=countdown)

upload/helpers.py

@@ -687,18 +687,6 @@ def parse_headers(headers, upload_params):
     return {"content_type": content_type, "reduced_redundancy": reduced_redundancy}
 
 
-def store_report_in_redis(request, commitid, reportid, redis):
-    encoding = request.META.get("HTTP_X_CONTENT_ENCODING") or request.META.get(
-        "HTTP_CONTENT_ENCODING"
-    )
-    redis_key = (
-        f"upload/{commitid[:7]}/{reportid}/{'gzip' if encoding == 'gzip' else 'plain'}"
-    )
-    redis.setex(redis_key, 10800, request.body)
-
-    return redis_key
-
-
 def dispatch_upload_task(
     task_arguments,
     repository,
@@ -746,6 +734,7 @@ def dispatch_upload_task(
         commitid=commitid,
         report_type=str(report_type),
         report_code=task_arguments.get("report_code"),
+        arguments=task_arguments,
         countdown=max(
             countdown, int(get_config("setup", "upload_processing_delay") or 0)
         ),

upload/tests/test_upload.py

@@ -15,7 +15,8 @@
 from rest_framework import status
 from rest_framework.exceptions import NotFound, ValidationError
 from rest_framework.reverse import reverse
-from rest_framework.test import APIRequestFactory, APITestCase
+from rest_framework.test import APITestCase
+from shared.api_archive.archive import ArchiveService
 from shared.django_apps.core.tests.factories import OwnerFactory
 from shared.torngit.exceptions import (
     TorngitClientGeneralError,
@@ -37,7 +38,6 @@
     insert_commit,
     parse_headers,
     parse_params,
-    store_report_in_redis,
     validate_upload,
 )
 from upload.tokenless.tokenless import TokenlessUploadHandler
@@ -713,31 +713,6 @@ def test_parse_request_headers(self):
                 {"version": "v4"},
             ) == {"content_type": "text/plain", "reduced_redundancy": True}
 
-    def test_store_report_in_redis(self):
-        redis = MockRedis()
-
-        with self.subTest("gzip encoding"):
-            assert (
-                store_report_in_redis(
-                    APIRequestFactory().get("", HTTP_X_CONTENT_ENCODING="gzip"),
-                    "1c78206f1a46dc6db8412a491fc770eb7d0f8a47",
-                    "report",
-                    redis,
-                )
-                == "upload/1c78206/report/gzip"
-            )
-
-        with self.subTest("plain encoding"):
-            assert (
-                store_report_in_redis(
-                    APIRequestFactory().get(""),
-                    "1c78206f1a46dc6db8412a491fc770eb7d0f8a47",
-                    "report",
-                    redis,
-                )
-                == "upload/1c78206/report/plain"
-            )
-
     def test_validate_upload_repository_moved(self):
         redis = MockRedis()
         owner = G(Owner, plan="users-free")
@@ -893,9 +868,10 @@ def test_dispatch_upload_task(self, upload):
         upload.assert_called_once_with(
             repoid=repo.repoid,
             commitid=task_arguments.get("commit"),
+            report_type="coverage",
             report_code="local_report",
+            arguments=task_arguments,
             countdown=4,
-            report_type="coverage",
         )
 
 
@@ -986,6 +962,7 @@ def test_invalid_request_params_invalid_package(self):
 
         assert response.status_code == status.HTTP_400_BAD_REQUEST
 
+    @patch("shared.api_archive.archive.ArchiveService.write_file")
     @patch("upload.views.legacy.get_redis_connection")
     @patch("upload.views.legacy.uuid4")
     @patch("upload.views.legacy.dispatch_upload_task")
@@ -997,6 +974,7 @@ def test_successful_upload_v2(
         mock_dispatch_upload,
         mock_uuid4,
         mock_get_redis,
+        mock_write_file,
     ):
         class MockRepoProviderAdapter:
             async def get_commit(self, commit, token):
@@ -1033,6 +1011,14 @@ async def get_commit(self, commit, token):
         )
         assert headers["content-type"] != "text/plain"
 
+        archive_service = ArchiveService(self.repo)
+        datetime = timezone.now().strftime("%Y-%m-%d")
+        repo_hash = archive_service.get_archive_hash(self.repo)
+        expected_url = f"v4/raw/{datetime}/{repo_hash}/b521e55aef79b101f48e2544837ca99a7fa3bf6b/dec1f00b-1883-40d0-afd6-6dcb876510be.txt"
+
+        mock_write_file.assert_called_with(
+            expected_url, b"coverage report", is_already_gzipped=False
+        )
         assert mock_dispatch_upload.call_args[0][0] == {
             "commit": "b521e55aef79b101f48e2544837ca99a7fa3bf6b",
             "token": "a03e5d02-9495-4413-b0d8-05651bb2e842",
@@ -1045,8 +1031,7 @@ async def get_commit(self, commit, token):
             "build_url": None,
             "branch": None,
             "reportid": "dec1f00b-1883-40d0-afd6-6dcb876510be",
-            "redis_key": "upload/b521e55/dec1f00b-1883-40d0-afd6-6dcb876510be/plain",
-            "url": None,
+            "url": expected_url,
             "job": None,
         }
 
@@ -1060,6 +1045,7 @@ async def get_commit(self, commit, token):
             == "https://app.codecov.io/github/codecovtest/upload-test-repo/commit/b521e55aef79b101f48e2544837ca99a7fa3bf6b"
         )
 
+    @patch("shared.api_archive.archive.ArchiveService.write_file")
     @patch("upload.views.legacy.get_redis_connection")
     @patch("upload.views.legacy.uuid4")
     @patch("upload.views.legacy.dispatch_upload_task")
@@ -1071,6 +1057,7 @@ def test_successful_upload_v2_slash(
         mock_dispatch_upload,
         mock_uuid4,
         mock_get_redis,
+        mock_write_file,
     ):
         class MockRepoProviderAdapter:
             async def get_commit(self, commit, token):
@@ -1107,6 +1094,14 @@ async def get_commit(self, commit, token):
         )
         assert headers["content-type"] != "text/plain"
 
+        archive_service = ArchiveService(self.repo)
+        datetime = timezone.now().strftime("%Y-%m-%d")
+        repo_hash = archive_service.get_archive_hash(self.repo)
+        expected_url = f"v4/raw/{datetime}/{repo_hash}/b521e55aef79b101f48e2544837ca99a7fa3bf6b/dec1f00b-1883-40d0-afd6-6dcb876510be.txt"
+
+        mock_write_file.assert_called_with(
+            expected_url, b"coverage report", is_already_gzipped=False
+        )
         assert mock_dispatch_upload.call_args[0][0] == {
             "commit": "b521e55aef79b101f48e2544837ca99a7fa3bf6b",
             "token": "a03e5d02-9495-4413-b0d8-05651bb2e842",
@@ -1119,8 +1114,7 @@ async def get_commit(self, commit, token):
             "build_url": None,
             "branch": None,
             "reportid": "dec1f00b-1883-40d0-afd6-6dcb876510be",
-            "redis_key": "upload/b521e55/dec1f00b-1883-40d0-afd6-6dcb876510be/plain",
-            "url": None,
+            "url": expected_url,
             "job": None,
         }
 

upload/tests/views/test_bundle_analysis.py

@@ -87,9 +87,10 @@ def test_upload_bundle_analysis_success(db, client, mocker, mock_redis):
     upload.assert_called_with(
         commitid=commit_sha,
         repoid=repository.repoid,
-        countdown=4,
         report_code=None,
         report_type="bundle_analysis",
+        arguments=ANY,
+        countdown=4,
     )
     mock_metrics.assert_called_with(
         **{
@@ -176,9 +177,10 @@ def test_upload_bundle_analysis_success_shelter(db, client, mocker, mock_redis):
     upload.assert_called_with(
         commitid=commit_sha,
         repoid=repository.repoid,
-        countdown=4,
         report_code=None,
         report_type="bundle_analysis",
+        arguments=ANY,
+        countdown=4,
     )
     mock_metrics.assert_called_with(
         **{
@@ -260,9 +262,10 @@ def test_upload_bundle_analysis_existing_commit(db, client, mocker, mock_redis):
     upload.assert_called_with(
         commitid=commit.commitid,
         repoid=repository.repoid,
-        countdown=4,
         report_code=None,
         report_type="bundle_analysis",
+        arguments=ANY,
+        countdown=4,
     )
     mock_metrics.assert_called_with(
         **{

upload/tests/views/test_test_results.py

@@ -74,8 +74,10 @@ def test_upload_test_results(db, client, mocker, mock_redis):
 
     # saves args in Redis
     redis = get_redis_connection()
-    args = redis.rpop(f"uploads/{repository.repoid}/{commit_sha}/test_results")
-    assert json.loads(args) == {
+    args = json.loads(
+        redis.rpop(f"uploads/{repository.repoid}/{commit_sha}/test_results")
+    )
+    assert args == {
         "reportid": reportid,
         "build": "test-build",
         "build_url": "test-build-url",
@@ -95,9 +97,10 @@ def test_upload_test_results(db, client, mocker, mock_redis):
     upload.assert_called_with(
         commitid=commit_sha,
         repoid=repository.repoid,
-        countdown=4,
         report_code=None,
         report_type="test_results",
+        arguments=args,
+        countdown=4,
     )
     mock_prometheus_metrics.assert_called_with(
         **{

upload/views/legacy.py

@@ -35,7 +35,6 @@
     insert_commit,
     parse_headers,
     parse_params,
-    store_report_in_redis,
     validate_upload,
 )
 from upload.metrics import API_UPLOAD_COUNTER
@@ -201,8 +200,9 @@ def post(self, request, *args, **kwargs):
         # --------- Handle the actual upload
 
         reportid = str(uuid4())
-        path = None  # populated later for v4 uploads when generating presigned PUT url
-        redis_key = None  # populated later for v2 uploads when storing report in Redis
+        # populated later for `v4` uploads when generating presigned PUT url,
+        # or by `v2` uploads when storing the report directly
+        path = None
 
         # Get the url where the commit details can be found on the Codecov site, we'll return this in the response
         destination_url = f"{settings.CODECOV_DASHBOARD_URL}/{owner.service}/{owner.username}/{repository.name}/commit/{commitid}"
@@ -212,7 +212,7 @@ def post(self, request, *args, **kwargs):
         repo_hash = archive_service.get_archive_hash(repository)
         default_path = f"v4/raw/{datetime}/{repo_hash}/{commitid}/{reportid}.txt"
 
-        # v2 - store request body in redis
+        # v2 - store request body directly
         if version == "v2":
             log.info(
                 "Started V2 upload",
@@ -224,15 +224,22 @@ def post(self, request, *args, **kwargs):
                     upload_params=upload_params,
                 ),
             )
-            redis_key = store_report_in_redis(request, commitid, reportid, redis)
+
+            path = default_path
+            encoding = request.META.get("HTTP_X_CONTENT_ENCODING") or request.META.get(
+                "HTTP_CONTENT_ENCODING"
+            )
+            archive_service.write_file(
+                path, request.body, is_already_gzipped=(encoding == "gzip")
+            )
 
             log.info(
-                "Stored coverage report in redis",
+                "Stored coverage report",
                 extra=dict(
                     commit=commitid,
                     upload_params=upload_params,
                     reportid=reportid,
-                    redis_key=redis_key,
+                    path=path,
                     repoid=repository.repoid,
                 ),
             )
@@ -271,6 +278,10 @@ def post(self, request, *args, **kwargs):
                 path = default_path
 
             try:
+                # When using shelter (`is_shelter_request`), the returned `upload_url` is being
+                # ignored, as shelter is handling the creation of a "presigned put" matching the
+                # `storage_path`.
+                # This code runs here just for backwards compatibility reasons:
                 upload_url = archive_service.create_presigned_put(default_path)
             except Exception as e:
                 log.warning(
@@ -311,10 +322,9 @@ def post(self, request, *args, **kwargs):
             **queue_params,
             "build_url": build_url,
             "reportid": reportid,
-            "redis_key": redis_key,  # location of report for v2 uploads; this will be "None" for v4 uploads
             "url": (
                 path
-                if path  # If a path was generated for a v4 upload, pass that to the 'url' field, potentially overwriting it
+                if path  # If a path was generated for an upload, pass that to the 'url' field, potentially overwriting it
                 else upload_params.get("url")
             ),
             # These values below might be different from the initial request parameters, so overwrite them here to ensure they're up-to-date