feat: Improve upload storage path generation for reports #1119

trent-codecov · 2025-01-31T19:18:03Z

Purpose/Motivation

What is the feature? Why is this being done?

Links to relevant tickets

What does this PR do?

Include a brief description of the changes in this PR. Bullet points are your friend.

Notes to Reviewer

Anything to note to the team? Any tips on how to review, or where to start?

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

upload/views/uploads.py

codecov · 2025-01-31T19:40:45Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.07%. Comparing base (7e5a947) to head (cfdb5fa).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1119   +/-   ##
=======================================
  Coverage   96.07%   96.07%           
=======================================
  Files         837      837           
  Lines       19656    19657    +1     
=======================================
+ Hits        18885    18886    +1     
  Misses        771      771

Flag	Coverage Δ
unit	`95.98% <100.00%> (+<0.01%)`	⬆️
unit-latest-uploader	`95.98% <100.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

codecov-staging · 2025-01-31T19:40:47Z

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

codecov-qa · 2025-01-31T19:40:53Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.98%. Comparing base (7e5a947) to head (cfdb5fa).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found.

codecov-public-qa · 2025-01-31T19:41:02Z

❌ 31 Tests Failed:

Tests completed	Failed	Passed	Skipped
2716	31	2685	6

View the top 3 failed tests by shortest run time

upload/tests/views/test_uploads.py::::test_uploads_post_token_required_auth_check[False-branch-fork:branch-False]

Stack Traces | 0.083s run time

db = None, mocker = <pytest_mock.plugin.MockerFixture object at 0x7fe3b6dca600>
mock_redis = FakeStrictRedis<ConnectionPool<FakeConnection<server=<fakeredis._server.FakeServer object at 0x7fe3c03eb590>,db=0>>>
private = False, branch = 'fork:branch', branch_sent = 'branch'
upload_token_required_for_public_repos = False

    @pytest.mark.parametrize("private", [False, True])
    @pytest.mark.parametrize("branch", ["branch", "fork:branch", "someone/fork:branch"])
    @pytest.mark.parametrize(
        "branch_sent", [None, "branch", "fork:branch", "someone/fork:branch"]
    )
    @pytest.mark.parametrize("upload_token_required_for_public_repos", [True, False])
    def test_uploads_post_token_required_auth_check(
        db,
        mocker,
        mock_redis,
        private,
        branch,
        branch_sent,
        upload_token_required_for_public_repos,
    ):
        presigned_put_mock = mocker.patch(
            "shared.api_archive.archive.StorageService.create_presigned_put",
            return_value="presigned put",
        )
        upload_task_mock = mocker.patch(
            "upload.views.uploads.trigger_upload_task", return_value=True
        )
        analytics_service_mock = mocker.patch("upload.views.uploads.AnalyticsService")
    
        repository = RepositoryFactory(
            name="the_repo",
            author__username="codecov",
            author__service="github",
            private=private,
            author__upload_token_required_for_public_repos=upload_token_required_for_public_repos,
        )
        commit = CommitFactory(repository=repository)
        commit.branch = branch
        commit_report = CommitReport.objects.create(commit=commit, code="code")
        repository.save()
        commit_report.save()
        commit.save()
    
        client = APIClient()
        url = reverse(
            "new_upload.uploads",
            args=[
                "github",
                "codecov::::the_repo",
                commit.commitid,
                commit_report.code,
            ],
        )
        if branch_sent is not None:
            data = {
                "state": "uploaded",
                "flags": ["flag1", "flag2"],
                "version": "version",
                "branch": branch_sent,
            }
        else:
            data = {
                "state": "uploaded",
                "flags": ["flag1", "flag2"],
                "version": "version",
            }
>       response = client.post(
            url,
            data,
        )

.../tests/views/test_uploads.py:500: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
.../local/lib/python3.12.............../site-packages/rest_framework/test.py:295: in post
    response = super().post(
.../local/lib/python3.12.............../site-packages/rest_framework/test.py:209: in post
    return self.generic('POST', path, data, content_type, **extra)
.../local/lib/python3.12.............../site-packages/rest_framework/test.py:233: in generic
    return super().generic(
.../local/lib/python3.12.../django/test/client.py:609: in generic
    return self.request(**r)
.../local/lib/python3.12.............../site-packages/rest_framework/test.py:285: in request
    return super().request(**kwargs)
.../local/lib/python3.12.............../site-packages/rest_framework/test.py:237: in request
    request = super().request(**kwargs)
.../local/lib/python3.12.../django/test/client.py:891: in request
    self.check_exception(response)
.../local/lib/python3.12.../django/test/client.py:738: in check_exception
    raise exc_value
.../local/lib/python3.12.../core/handlers/exception.py:55: in inner
    response = get_response(request)
.../local/lib/python3.12.../core/handlers/base.py:197: in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
.../local/lib/python3.12.../integrations/django/views.py:89: in sentry_wrapped_callback
    return callback(request, *args, **kwargs)
.../local/lib/python3.12.../views/decorators/csrf.py:56: in wrapper_view
    return view_func(*args, **kwargs)
.../local/lib/python3.12.../views/generic/base.py:104: in view
    return self.dispatch(request, *args, **kwargs)
.../local/lib/python3.12............/site-packages/rest_framework/views.py:509: in dispatch
    response = self.handle_exception(exc)
.../local/lib/python3.12............/site-packages/rest_framework/views.py:469: in handle_exception
    self.raise_uncaught_exception(exc)
.../local/lib/python3.12............/site-packages/rest_framework/views.py:480: in raise_uncaught_exception
    raise exc
.../local/lib/python3.12............/site-packages/rest_framework/views.py:506: in dispatch
    response = handler(request, *args, **kwargs)
.../local/lib/python3.12.../site-packages/rest_framework/generics.py:246: in post
    return self.create(request, *args, **kwargs)
.../local/lib/python3.12.../site-packages/rest_framework/mixins.py:19: in create
    self.perform_create(serializer)
upload/views/uploads.py:239: in perform_create
    instance = create_upload(
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

serializer = UploadSerializer(context={'request': <rest_framework.request.Request: POST '.../upload/github/codecov::::the_repo/commits...  storage_path = CharField(required=False, write_only=True)
    ci_service = CharField(required=False, write_only=True)
repository = <Repository: Repo<Owner<github/codecov>/the_repo>>
commit = <Commit: Commit object (2105)>
report = <CommitReport: CommitReport object (851)>, is_shelter_request = None
analytics_token = 'tokenless_upload'

    def create_upload(
        serializer: UploadSerializer,
        repository: Repository,
        commit: Commit,
        report: CommitReport,
        is_shelter_request: bool,
        analytics_token: str,
    ) -> ReportSession:
        version = (
            serializer.validated_data["version"]
            if "version" in serializer.validated_data
            else None
        )
        archive_service = ArchiveService(repository)
        # only Shelter requests are allowed to set their own `storage_path`
    
>       if serializer.validated_data["storage_path"] is None or not is_shelter_request:
E       KeyError: 'storage_path'

upload/views/uploads.py:62: KeyError

upload/tests/views/test_uploads.py::::test_uploads_post_token_required_auth_check[False-fork:branch-someone/fork:branch-False]

Stack Traces | 0.083s run time

db = None, mocker = <pytest_mock.plugin.MockerFixture object at 0x7fe3b5b3b230>
mock_redis = FakeStrictRedis<ConnectionPool<FakeConnection<server=<fakeredis._server.FakeServer object at 0x7fe3b5b722d0>,db=0>>>
private = False, branch = 'someone/fork:branch', branch_sent = 'fork:branch'
upload_token_required_for_public_repos = False

    @pytest.mark.parametrize("private", [False, True])
    @pytest.mark.parametrize("branch", ["branch", "fork:branch", "someone/fork:branch"])
    @pytest.mark.parametrize(
        "branch_sent", [None, "branch", "fork:branch", "someone/fork:branch"]
    )
    @pytest.mark.parametrize("upload_token_required_for_public_repos", [True, False])
    def test_uploads_post_token_required_auth_check(
        db,
        mocker,
        mock_redis,
        private,
        branch,
        branch_sent,
        upload_token_required_for_public_repos,
    ):
        presigned_put_mock = mocker.patch(
            "shared.api_archive.archive.StorageService.create_presigned_put",
            return_value="presigned put",
        )
        upload_task_mock = mocker.patch(
            "upload.views.uploads.trigger_upload_task", return_value=True
        )
        analytics_service_mock = mocker.patch("upload.views.uploads.AnalyticsService")
    
        repository = RepositoryFactory(
            name="the_repo",
            author__username="codecov",
            author__service="github",
            private=private,
            author__upload_token_required_for_public_repos=upload_token_required_for_public_repos,
        )
        commit = CommitFactory(repository=repository)
        commit.branch = branch
        commit_report = CommitReport.objects.create(commit=commit, code="code")
        repository.save()
        commit_report.save()
        commit.save()
    
        client = APIClient()
        url = reverse(
            "new_upload.uploads",
            args=[
                "github",
                "codecov::::the_repo",
                commit.commitid,
                commit_report.code,
            ],
        )
        if branch_sent is not None:
            data = {
                "state": "uploaded",
                "flags": ["flag1", "flag2"],
                "version": "version",
                "branch": branch_sent,
            }
        else:
            data = {
                "state": "uploaded",
                "flags": ["flag1", "flag2"],
                "version": "version",
            }
>       response = client.post(
            url,
            data,
        )

.../tests/views/test_uploads.py:500: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
.../local/lib/python3.12.............../site-packages/rest_framework/test.py:295: in post
    response = super().post(
.../local/lib/python3.12.............../site-packages/rest_framework/test.py:209: in post
    return self.generic('POST', path, data, content_type, **extra)
.../local/lib/python3.12.............../site-packages/rest_framework/test.py:233: in generic
    return super().generic(
.../local/lib/python3.12.../django/test/client.py:609: in generic
    return self.request(**r)
.../local/lib/python3.12.............../site-packages/rest_framework/test.py:285: in request
    return super().request(**kwargs)
.../local/lib/python3.12.............../site-packages/rest_framework/test.py:237: in request
    request = super().request(**kwargs)
.../local/lib/python3.12.../django/test/client.py:891: in request
    self.check_exception(response)
.../local/lib/python3.12.../django/test/client.py:738: in check_exception
    raise exc_value
.../local/lib/python3.12.../core/handlers/exception.py:55: in inner
    response = get_response(request)
.../local/lib/python3.12.../core/handlers/base.py:197: in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
.../local/lib/python3.12.../integrations/django/views.py:89: in sentry_wrapped_callback
    return callback(request, *args, **kwargs)
.../local/lib/python3.12.../views/decorators/csrf.py:56: in wrapper_view
    return view_func(*args, **kwargs)
.../local/lib/python3.12.../views/generic/base.py:104: in view
    return self.dispatch(request, *args, **kwargs)
.../local/lib/python3.12............/site-packages/rest_framework/views.py:509: in dispatch
    response = self.handle_exception(exc)
.../local/lib/python3.12............/site-packages/rest_framework/views.py:469: in handle_exception
    self.raise_uncaught_exception(exc)
.../local/lib/python3.12............/site-packages/rest_framework/views.py:480: in raise_uncaught_exception
    raise exc
.../local/lib/python3.12............/site-packages/rest_framework/views.py:506: in dispatch
    response = handler(request, *args, **kwargs)
.../local/lib/python3.12.../site-packages/rest_framework/generics.py:246: in post
    return self.create(request, *args, **kwargs)
.../local/lib/python3.12.../site-packages/rest_framework/mixins.py:19: in create
    self.perform_create(serializer)
upload/views/uploads.py:239: in perform_create
    instance = create_upload(
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

serializer = UploadSerializer(context={'request': <rest_framework.request.Request: POST '.../upload/github/codecov::::the_repo/commits...  storage_path = CharField(required=False, write_only=True)
    ci_service = CharField(required=False, write_only=True)
repository = <Repository: Repo<Owner<github/codecov>/the_repo>>
commit = <Commit: Commit object (2113)>
report = <CommitReport: CommitReport object (859)>, is_shelter_request = None
analytics_token = 'tokenless_upload'

    def create_upload(
        serializer: UploadSerializer,
        repository: Repository,
        commit: Commit,
        report: CommitReport,
        is_shelter_request: bool,
        analytics_token: str,
    ) -> ReportSession:
        version = (
            serializer.validated_data["version"]
            if "version" in serializer.validated_data
            else None
        )
        archive_service = ArchiveService(repository)
        # only Shelter requests are allowed to set their own `storage_path`
    
>       if serializer.validated_data["storage_path"] is None or not is_shelter_request:
E       KeyError: 'storage_path'

upload/views/uploads.py:62: KeyError

upload/tests/views/test_uploads.py::::test_uploads_post_github_oidc_auth

Stack Traces | 0.084s run time

mock_jwks_client = <MagicMock name='PyJWKClient' id='140617202246304'>
mock_jwt_decode = <MagicMock name='decode' id='140617208289648'>
analytics_service_mock = <MagicMock name='AnalyticsService' id='140616563354240'>
db = None, mocker = <pytest_mock.plugin.MockerFixture object at 0x7fe3c01cae40>
mock_redis = FakeStrictRedis<ConnectionPool<FakeConnection<server=<fakeredis._server.FakeServer object at 0x7fe3fde8c170>,db=0>>>

    @patch("upload.views.uploads.AnalyticsService")
    @patch("upload.helpers.jwt.decode")
    @patch("upload.helpers.PyJWKClient")
    def test_uploads_post_github_oidc_auth(
        mock_jwks_client,
        mock_jwt_decode,
        analytics_service_mock,
        db,
        mocker,
        mock_redis,
    ):
        presigned_put_mock = mocker.patch(
            "shared.api_archive.archive.StorageService.create_presigned_put",
            return_value="presigned put",
        )
        upload_task_mock = mocker.patch(
            "upload.views.uploads.trigger_upload_task", return_value=True
        )
    
        repository = RepositoryFactory(
            name="the_repo",
            author__username="codecov",
            author__service="github",
            author__upload_token_required_for_public_repos=True,
            private=False,
        )
        mock_jwt_decode.return_value = {
            "repository": f"url/{repository.name}",
            "repository_owner": repository.author.username,
            "iss": "https://token.actions.githubusercontent.com",
            "audience": [settings.CODECOV_API_URL],
        }
        token = "ThisValueDoesNotMatterBecauseOf_mock_jwt_decode"
    
        commit = CommitFactory(repository=repository)
        commit_report = CommitReport.objects.create(commit=commit, code="code")
    
        client = APIClient()
        url = reverse(
            "new_upload.uploads",
            args=[
                "github",
                "codecov::::the_repo",
                commit.commitid,
                commit_report.code,
            ],
        )
>       response = client.post(
            url,
            {
                "state": "uploaded",
                "flags": ["flag1", "flag2"],
                "version": "version",
            },
            headers={"Authorization": f"token {token}"},
        )

.../tests/views/test_uploads.py:638: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
.../local/lib/python3.12.............../site-packages/rest_framework/test.py:295: in post
    response = super().post(
.../local/lib/python3.12.............../site-packages/rest_framework/test.py:209: in post
    return self.generic('POST', path, data, content_type, **extra)
.../local/lib/python3.12.............../site-packages/rest_framework/test.py:233: in generic
    return super().generic(
.../local/lib/python3.12.../django/test/client.py:609: in generic
    return self.request(**r)
.../local/lib/python3.12.............../site-packages/rest_framework/test.py:285: in request
    return super().request(**kwargs)
.../local/lib/python3.12.............../site-packages/rest_framework/test.py:237: in request
    request = super().request(**kwargs)
.../local/lib/python3.12.../django/test/client.py:891: in request
    self.check_exception(response)
.../local/lib/python3.12.../django/test/client.py:738: in check_exception
    raise exc_value
.../local/lib/python3.12.../core/handlers/exception.py:55: in inner
    response = get_response(request)
.../local/lib/python3.12.../core/handlers/base.py:197: in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
.../local/lib/python3.12.../integrations/django/views.py:89: in sentry_wrapped_callback
    return callback(request, *args, **kwargs)
.../local/lib/python3.12.../views/decorators/csrf.py:56: in wrapper_view
    return view_func(*args, **kwargs)
.../local/lib/python3.12.../views/generic/base.py:104: in view
    return self.dispatch(request, *args, **kwargs)
.../local/lib/python3.12............/site-packages/rest_framework/views.py:509: in dispatch
    response = self.handle_exception(exc)
.../local/lib/python3.12............/site-packages/rest_framework/views.py:469: in handle_exception
    self.raise_uncaught_exception(exc)
.../local/lib/python3.12............/site-packages/rest_framework/views.py:480: in raise_uncaught_exception
    raise exc
.../local/lib/python3.12............/site-packages/rest_framework/views.py:506: in dispatch
    response = handler(request, *args, **kwargs)
.../local/lib/python3.12.../site-packages/rest_framework/generics.py:246: in post
    return self.create(request, *args, **kwargs)
.../local/lib/python3.12.../site-packages/rest_framework/mixins.py:19: in create
    self.perform_create(serializer)
upload/views/uploads.py:239: in perform_create
    instance = create_upload(
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

serializer = UploadSerializer(context={'request': <rest_framework.request.Request: POST '.../upload/github/codecov::::the_repo/commits...  storage_path = CharField(required=False, write_only=True)
    ci_service = CharField(required=False, write_only=True)
repository = <Repository: Repo<Owner<github/codecov>/the_repo>>
commit = <Commit: Commit object (2121)>
report = <CommitReport: CommitReport object (867)>, is_shelter_request = None
analytics_token = 'oidc_token_upload'

    def create_upload(
        serializer: UploadSerializer,
        repository: Repository,
        commit: Commit,
        report: CommitReport,
        is_shelter_request: bool,
        analytics_token: str,
    ) -> ReportSession:
        version = (
            serializer.validated_data["version"]
            if "version" in serializer.validated_data
            else None
        )
        archive_service = ArchiveService(repository)
        # only Shelter requests are allowed to set their own `storage_path`
    
>       if serializer.validated_data["storage_path"] is None or not is_shelter_request:
E       KeyError: 'storage_path'

upload/views/uploads.py:62: KeyError

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

github-actions · 2025-01-31T19:41:05Z

✅ All tests successful. No failed tests were found.

📣 Thoughts on this report? Let Codecov know! | Powered by Codecov

feat: Improve upload storage path generation for reports

c9ca88f

trent-codecov requested a review from a team as a code owner January 31, 2025 19:18

adrian-codecov reviewed Jan 31, 2025

View reviewed changes

upload/views/uploads.py Outdated Show resolved Hide resolved

fix: Conditionally generate external_id for uploads

7b1d26a

adrian-codecov approved these changes Jan 31, 2025

View reviewed changes

fix: null check for validated data

cfdb5fa

trent-codecov added this pull request to the merge queue Jan 31, 2025

Merged via the queue into main with commit e4924ba Jan 31, 2025
19 checks passed

trent-codecov deleted the trent/save-report-once branch January 31, 2025 20:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: Improve upload storage path generation for reports #1119

feat: Improve upload storage path generation for reports #1119

Uh oh!

trent-codecov commented Jan 31, 2025

Uh oh!

Uh oh!

codecov bot commented Jan 31, 2025 •

edited

Loading

Uh oh!

codecov-staging bot commented Jan 31, 2025 •

edited by codecov-notifications bot

Loading

Uh oh!

codecov-qa bot commented Jan 31, 2025 •

edited

Loading

Uh oh!

codecov-public-qa bot commented Jan 31, 2025

Uh oh!

github-actions bot commented Jan 31, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

feat: Improve upload storage path generation for reports #1119

feat: Improve upload storage path generation for reports #1119

Uh oh!

Conversation

trent-codecov commented Jan 31, 2025

Purpose/Motivation

Links to relevant tickets

What does this PR do?

Notes to Reviewer

Legal Boilerplate

Uh oh!

Uh oh!

codecov bot commented Jan 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

codecov-staging bot commented Jan 31, 2025 • edited by codecov-notifications bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

codecov-qa bot commented Jan 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

codecov-public-qa bot commented Jan 31, 2025

❌ 31 Tests Failed:

Uh oh!

github-actions bot commented Jan 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Jan 31, 2025 •

edited

Loading

codecov-staging bot commented Jan 31, 2025 •

edited by codecov-notifications bot

Loading

codecov-qa bot commented Jan 31, 2025 •

edited

Loading

github-actions bot commented Jan 31, 2025 •

edited

Loading