bundle analysis: compute measurements for unknown asset types #885

JerrySentry · 2024-10-15T08:32:57Z

Computes the trends measurements for asset types with unknown types, do this buy subtracting all the known bundle type sizes from the total bundle size for each time interval of the measurements data.

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.

codecov-staging · 2024-10-15T08:44:21Z

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

codecov · 2024-10-15T08:44:26Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.32%. Comparing base (3c2b4ff) to head (1d5641a).
Report is 3 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #885   +/-   ##
=======================================
  Coverage   96.31%   96.32%           
=======================================
  Files         823      823           
  Lines       19084    19110   +26     
=======================================
+ Hits        18381    18407   +26     
  Misses        703      703

Flag	Coverage Δ
unit	`92.67% <100.00%> (+0.01%)`	⬆️
unit-latest-uploader	`92.67% <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-public-qa · 2024-10-15T08:44:28Z

Test Failures Detected: Due to failing tests, we cannot provide coverage reports at this time.

❌ Failed Test Results:

Completed 2682 tests with 1 failed, 2675 passed and 6 skipped.

View the full list of failed tests

pytest

Class name: api.internal.tests.test_charts.TestChartQueryRunnerQuery
Test name: test_query_aggregates_multiple_repository_totals

self = <tests.test_charts.TestChartQueryRunnerQuery testMethod=test_query_aggregates_multiple_repository_totals>

    @override_settings(GITHUB_CLIENT_ID="3d44be0e772666136a13")
    def test_query_aggregates_multiple_repository_totals(self):
        query_runner = ChartQueryRunner(
            user=self.user,
            request_params={
                "owner_username": self.org.username,
                "service": self.org.service,
                "end_date": str(timezone.now()),
                "grouping_unit": "day",
            },
        )
    
        results = query_runner.run_query()
    
        assert len(results) == 1
>       assert results[0]["total_hits"] == 114
E       AssertionError: assert Decimal('14') == 114

.../internal/tests/test_charts.py:564: AssertionError

codecov-qa · 2024-10-15T08:47:25Z

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
2676	1	2675	6

View the top 1 failed tests by shortest run time

api.internal.tests.test_charts.TestChartQueryRunnerQuery test_query_aggregates_multiple_repository_totals

Stack Traces | 0.067s run time

self = &lt;tests.test_charts.TestChartQueryRunnerQuery testMethod=test_query_aggregates_multiple_repository_totals&gt;

    @override_settings(GITHUB_CLIENT_ID="3d44be0e772666136a13")
    def test_query_aggregates_multiple_repository_totals(self):
        query_runner = ChartQueryRunner(
            user=self.user,
            request_params={
                "owner_username": self.org.username,
                "service": self.org.service,
                "end_date": str(timezone.now()),
                "grouping_unit": "day",
            },
        )
    
        results = query_runner.run_query()
    
        assert len(results) == 1
&gt;       assert results[0]["total_hits"] == 114
E       AssertionError: assert Decimal('14') == 114

.../internal/tests/test_charts.py:564: AssertionError

To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard

suejung-sentry

approved this! had some questions for my own learning mostly

suejung-sentry · 2024-10-15T21:20:41Z

graphql_api/tests/test_bundle_analysis_measurements.py

-                                        __typename
-                                        ... on BundleAnalysisReport {
-                                            bundle(name: "super") {
+                                bundleAnalysisReport {


May need to pull in main to pick up @calvin-codecov 's update for the graphql nesting proj

suejung-sentry · 2024-10-15T21:34:49Z

services/bundle_analysis.py

@@ -81,7 +81,7 @@ class BundleAnalysisMeasurementData(object):
    def __init__(
        self,
        raw_measurements: List[dict],
-        asset_type: BundleAnalysisMeasurementsAssetType,
+        asset_type: Union[BundleAnalysisMeasurementsAssetType, str],


Out of curiosity, why do we choose to use a union instead of adding to the existing enum?

Yeah I was looking to add an entry to that, but once I started making the changes I realized too many things (a lot of it on the ingestion side) would need to be updated. So I thought this would be the least disruptive way of doing it.

suejung-sentry · 2024-10-15T21:43:01Z

graphql_api/types/bundle_analysis/base.py

@@ -36,6 +39,35 @@ def _find_index_by_cursor(assets: List, cursor: str) -> int:
    return -1


+def _compute_unknown_asset_size_raw_measurements(fetched_data: dict) -> List[dict]:


nit: are these the types?
input Dict[BundleAnalysisMeasurementsAssetType, List[BundleAnalysisMeasurementData]]
output List[BundleAnalysisMeasurementData]

suejung-sentry · 2024-10-15T21:59:07Z

graphql_api/types/bundle_analysis/base.py

+                    return []
+                unknown_raw_measurements[i]["min"] -= raw_measurements[i]["min"]
+                unknown_raw_measurements[i]["max"] -= raw_measurements[i]["max"]
+                unknown_raw_measurements[i]["avg"] -= raw_measurements[i]["avg"]


I see we're using min, max, avg fields to represent "total bundle size" basically (and the 3 fields have equal values), so doing this subtraction exercise makes sense. If we ever change this to being things other than the "total", we may need to change the math to take the min of everything for min and max of everything for max, etc. .. Guessing we don't expect that any time soon?

Yeah true it would suck if the math becomes harder in the future, especially percentage type values.. We might need to rethink an approach to do unknown asset types in the future if it comes to it.

…11_bam

bundle analysis: compute measurements for unknown asset types

16ad8e0

JerrySentry marked this pull request as ready for review October 15, 2024 09:00

JerrySentry requested a review from a team as a code owner October 15, 2024 09:00

suejung-sentry approved these changes Oct 15, 2024

View reviewed changes

JerrySentry and others added 5 commits October 16, 2024 13:33

Merge branch 'main' into oct_11_bam

ff910f7

update typing

2b36aa8

Merge branch 'oct_11_bam' of github.com:codecov/codecov-api into oct_…

c0dce64

…11_bam

update typing 2

205beb3

revert typing

1d5641a

JerrySentry added this pull request to the merge queue Oct 17, 2024

Merged via the queue into main with commit 3b84808 Oct 17, 2024
19 checks passed

JerrySentry deleted the oct_11_bam branch October 17, 2024 13:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bundle analysis: compute measurements for unknown asset types #885

bundle analysis: compute measurements for unknown asset types #885

JerrySentry commented Oct 15, 2024

codecov-staging bot commented Oct 15, 2024 •

edited by codecov-notifications bot

Loading

codecov bot commented Oct 15, 2024 •

edited

Loading

codecov-public-qa bot commented Oct 15, 2024

pytest

codecov-qa bot commented Oct 15, 2024

suejung-sentry left a comment

suejung-sentry Oct 15, 2024

suejung-sentry Oct 15, 2024

JerrySentry Oct 16, 2024

suejung-sentry Oct 15, 2024

suejung-sentry Oct 15, 2024

JerrySentry Oct 16, 2024

		@@ -36,6 +39,35 @@ def _find_index_by_cursor(assets: List, cursor: str) -> int:
		return -1


		def _compute_unknown_asset_size_raw_measurements(fetched_data: dict) -> List[dict]:

bundle analysis: compute measurements for unknown asset types #885

bundle analysis: compute measurements for unknown asset types #885

Conversation

JerrySentry commented Oct 15, 2024

Legal Boilerplate

codecov-staging bot commented Oct 15, 2024 • edited by codecov-notifications bot Loading

Codecov Report

codecov bot commented Oct 15, 2024 • edited Loading

Codecov Report

codecov-public-qa bot commented Oct 15, 2024

❌ Failed Test Results:

pytest

codecov-qa bot commented Oct 15, 2024

❌ 1 Tests Failed:

suejung-sentry left a comment

Choose a reason for hiding this comment

suejung-sentry Oct 15, 2024

Choose a reason for hiding this comment

suejung-sentry Oct 15, 2024

Choose a reason for hiding this comment

JerrySentry Oct 16, 2024

Choose a reason for hiding this comment

suejung-sentry Oct 15, 2024

Choose a reason for hiding this comment

suejung-sentry Oct 15, 2024

Choose a reason for hiding this comment

JerrySentry Oct 16, 2024

Choose a reason for hiding this comment

codecov-staging bot commented Oct 15, 2024 •

edited by codecov-notifications bot

Loading

codecov bot commented Oct 15, 2024 •

edited

Loading