Improve typing and optimize Comparison (#1283)

Author: Swatinem

api/shared/compare/serializers.py

@@ -131,7 +131,7 @@ def get_lines(self, segment: Segment) -> serializers.ListField:
 class ImpactedFileSegmentsSerializer(serializers.Serializer):
     segments = serializers.SerializerMethodField()
 
-    def get_segments(self, file_path: str) -> ImpactedFileSegmentSerializer:
+    def get_segments(self, file_path: str) -> list[ImpactedFileSegmentSerializer]:
         file_comparison = self.context["comparison"].get_file_comparison(
             file_path, with_src=True, bypass_max_diff=True
         )

graphql_api/actions/comparison.py

@@ -1,5 +1,3 @@
-from typing import Optional, Union
-
 from compare.models import CommitComparison
 from graphql_api.types.comparison.comparison import (
     MissingBaseReport,
@@ -9,8 +7,8 @@
 
 
 def validate_commit_comparison(
-    commit_comparison: Optional[CommitComparison],
-) -> Union[MissingBaseReport, MissingHeadReport, MissingComparison]:
+    commit_comparison: CommitComparison | None,
+) -> MissingBaseReport | MissingHeadReport | MissingComparison | None:
     if not commit_comparison:
         return MissingComparison()
 

graphql_api/types/commit/commit.py

@@ -118,8 +118,8 @@ def resolve_list_uploads(commit: Commit, info: GraphQLResolveInfo, **kwargs):
 @commit_bindable.field("compareWithParent")
 @sentry_sdk.trace
 async def resolve_compare_with_parent(
-    commit: Commit, info: GraphQLResolveInfo, **kwargs
-):
+    commit: Commit, info: GraphQLResolveInfo, **kwargs: Any
+) -> ComparisonReport | Any:
     if not commit.parent_commit_id:
         return MissingBaseCommit()
 
@@ -129,7 +129,6 @@ async def resolve_compare_with_parent(
     )
 
     comparison_error = validate_commit_comparison(commit_comparison=commit_comparison)
-
     if comparison_error:
         return comparison_error
 

graphql_api/types/comparison/comparison.py

@@ -1,5 +1,5 @@
 from asyncio import gather
-from typing import List, Optional
+from typing import Any, List, Optional
 
 import sentry_sdk
 from ariadne import ObjectType, UnionType
@@ -245,9 +245,7 @@ def resolve_flag_comparisons_count(
 @sync_to_async
 @sentry_sdk.trace
 def resolve_has_different_number_of_head_and_base_reports(
-    comparison: ComparisonReport,
-    info: GraphQLResolveInfo,
-    **kwargs,  # type: ignore
+    comparison: ComparisonReport, info: GraphQLResolveInfo, **kwargs: Any
 ) -> bool:
     # TODO: can we remove the need for `info.context["comparison"]` here?
     if "comparison" not in info.context:

graphql_api/types/impacted_file/impacted_file.py

@@ -1,9 +1,10 @@
 import hashlib
-from typing import List, Union
+from typing import List
 
 import sentry_sdk
 from ariadne import ObjectType, UnionType
 from asgiref.sync import sync_to_async
+from graphql import GraphQLResolveInfo
 from shared.reports.types import ReportTotals
 from shared.torngit.exceptions import TorngitClientError
 
@@ -67,8 +68,8 @@ def resolve_hashed_path(impacted_file: ImpactedFile, info) -> str:
 @sync_to_async
 @sentry_sdk.trace
 def resolve_segments(
-    impacted_file: ImpactedFile, info, filters=None
-) -> Union[UnknownPath, ProviderError, SegmentComparisons]:
+    impacted_file: ImpactedFile, info: GraphQLResolveInfo, filters: dict | None = None
+) -> SegmentComparisons | UnknownPath | ProviderError:
     if filters is None:
         filters = {}
     if "comparison" not in info.context:

graphql_api/types/pull/pull.py

@@ -76,7 +76,6 @@ async def resolve_compare_with_base(
     commit_comparison = await comparison_loader.load((pull.compared_to, pull.head))
 
     comparison_error = validate_commit_comparison(commit_comparison=commit_comparison)
-
     if comparison_error:
         return comparison_error
 

pyproject.toml

@@ -74,4 +74,4 @@ dev-dependencies = [
 ]
 
 [tool.uv.sources]
-shared = { git = "https://github.com/codecov/shared", rev = "a2350552f09580e26a9356b30353d02fe0832855" }
+shared = { git = "https://github.com/codecov/shared", rev = "940103554b6072f61ad8534fcbefdab217138295" }

services/comparison.py

@@ -9,6 +9,7 @@
 
 import minio
 import pytz
+import sentry_sdk
 import shared.reports.api_report_service as report_service
 from asgiref.sync import async_to_sync
 from django.db.models import Prefetch, QuerySet
@@ -756,7 +757,7 @@ def head_report_without_applied_diff(self):
         return report
 
     @cached_property
-    def has_different_number_of_head_and_base_sessions(self):
+    def has_different_number_of_head_and_base_sessions(self) -> bool:
         """
         This method checks if the head and the base have different number of sessions.
         It makes use of the head_report_without_applied_diff property instead of the
@@ -989,10 +990,10 @@ class ComparisonReport(object):
     on a `CommitComparison`
     """
 
-    commit_comparison: CommitComparison = None
+    commit_comparison: CommitComparison
 
     @cached_property
-    def files(self) -> List[ImpactedFile]:
+    def files(self) -> list[ImpactedFile]:
         if not self.commit_comparison.report_storage_path:
             return []
 
@@ -1001,29 +1002,29 @@ def files(self) -> List[ImpactedFile]:
             ImpactedFile.create(**data) for data in comparison_data.get("files", [])
         ]
 
-    def impacted_file(self, path: str) -> Optional[ImpactedFile]:
+    def impacted_file(self, path: str) -> ImpactedFile | None:
         for file in self.files:
             if file.head_name == path:
                 return file
 
-    @cached_property
-    def impacted_files(self) -> List[ImpactedFile]:
+    @property
+    def impacted_files(self) -> list[ImpactedFile]:
         return self.files
 
     @cached_property
-    def impacted_files_with_unintended_changes(self) -> List[ImpactedFile]:
+    def impacted_files_with_unintended_changes(self) -> list[ImpactedFile]:
         return [file for file in self.files if file.has_changes]
 
     @cached_property
-    def impacted_files_with_direct_changes(self) -> List[ImpactedFile]:
+    def impacted_files_with_direct_changes(self) -> list[ImpactedFile]:
         return [file for file in self.files if file.has_diff or not file.has_changes]
 
+    @sentry_sdk.trace
     def _fetch_raw_comparison_data(self) -> dict:
         """
         Fetches the raw comparison data from storage
         """
-        repository = self.commit_comparison.compare_commit.repository
-        archive_service = ArchiveService(repository)
+        archive_service = ArchiveService(repository=None)
         try:
             data = archive_service.read_file(self.commit_comparison.report_storage_path)
             return json.loads(data)

upload/serializers.py

@@ -51,8 +51,7 @@ class Meta:
     raw_upload_location = serializers.SerializerMethodField()
 
     def get_raw_upload_location(self, obj: ReportSession) -> str:
-        repo = obj.report.commit.repository
-        archive_service = ArchiveService(repo)
+        archive_service = ArchiveService(repository=None)
         return archive_service.create_presigned_put(obj.storage_path)
 
     def get_url(self, obj: ReportSession) -> str: