Bundle Analysis: update asset model #227
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #227 +/- ##
==========================================
- Coverage 89.52% 89.48% -0.05%
==========================================
Files 325 324 -1
Lines 10424 10374 -50
Branches 1906 1904 -2
==========================================
- Hits 9332 9283 -49
+ Misses 1023 1020 -3
- Partials 69 71 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -183,6 +200,8 @@ def _parse_assets_event(self, prefix: str, event: str, value: str): | |||
| name=self.asset.name, | |||
| normalized_name=self.asset.normalized_name, | |||
| size=self.asset.size, | |||
| uuid=str(uuid.uuid4()), | |||
There was a problem hiding this comment.
Can you clarify how the uuid would be the same across commits if we are (apparently) creating random uuids?
There was a problem hiding this comment.
You are right, this is just the part 1 of the change to make linking assets across different commits work. By default we will create the uuid for an asset for this PR, for the next PR as the next step of the parsing process it will look at the previous commit's asset, if the algorithm thinks its the same then it will update the current asset's uuid to be the same as the previous, otherwise it will remain the same.
I choose to initialize an uuid on creation and then update it in the following parsing step is because the parsing code is already kind of convoluted, so setting the final uuid right now it would be a nightmare.
shared/bundle_analysis/utils.py
Outdated
| # Remove the dot in the extension | ||
| file_extension = file_extension[1:] | ||
| # At times file can be something like './index.js?module', remove the ? | ||
| if "?" in file_extension: |
There was a problem hiding this comment.
isn't this doing 2 passes for essentially the same thing? At least in JS, if we just did file_extension.split("?")[0] and there was no "?" it would return the same thing so I figure python would be the same.
There was a problem hiding this comment.
Aha true! updated.
| def _asset_type(self, name: str) -> AssetType: | ||
| extension = get_extension(name) | ||
|
|
||
| if extension in ["js"]: |
There was a problem hiding this comment.
just curious, where did we compile this list of extensions?
There was a problem hiding this comment.
this matches how Sentry buckets its asset types.
Or defined here https://www.notion.so/sentry/Type-Filtering-and-Grouping-82d5db9fd7f84737a4792df2ae30c22f
There was a problem hiding this comment.
perf! Thanks, exactly what I was looking for
Adds 2 columns to the asset table:
closes: codecov/engineering-team#1767
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.