Store v2 reports in GCS instead of Redis

Worker has some dedicated code to move an uploaded report from Redis over to GCS.
Instead of having that code, we can just store the report into GCS in the first place.

It also forwards all the task arguments directly to the `Upload` task, in addition to storing it in Redis, to be forward compatible to some more planned upload processing improvements.

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