-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bundle Analysis: Display comparison for file paths on PR comments #952
Changes from 6 commits
44588b1
ac1ae0a
60be280
800e91c
ed09cda
96ba584
44f01de
ebc82d3
0c365fd
c622488
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,7 @@ | |
import sentry_sdk | ||
from asgiref.sync import async_to_sync | ||
from django.template import loader | ||
from shared.bundle_analysis import BundleAnalysisComparison, BundleChange | ||
from shared.bundle_analysis import BundleAnalysisComparison, BundleChange, RouteChange | ||
from shared.torngit.exceptions import TorngitClientError | ||
from shared.validation.types import BundleThreshold | ||
|
||
|
@@ -34,6 +34,14 @@ class BundleRow(TypedDict): | |
is_change_outside_threshold: bool | ||
|
||
|
||
class BundleRouteRow(TypedDict): | ||
route_name: str | ||
change_size_readable: str | ||
percentage_change_readable: str | ||
change_icon: str | ||
route_size: str | ||
|
||
|
||
class BundleCommentTemplateContext(TypedDict): | ||
pull_url: str | ||
total_size_delta: int | ||
|
@@ -42,6 +50,7 @@ class BundleCommentTemplateContext(TypedDict): | |
status_level: Literal["INFO"] | Literal["WARNING"] | Literal["ERROR"] | ||
warning_threshold_readable: str | ||
bundle_rows: list[BundleRow] | ||
bundle_route_data: dict[str, list[BundleRouteRow]] | ||
has_cached_bundles: bool | ||
|
||
|
||
|
@@ -70,13 +79,17 @@ def build_default_message( | |
bundle_rows = self._create_bundle_rows( | ||
context.bundle_analysis_comparison, warning_threshold | ||
) | ||
bundle_route_data = self._create_bundle_route_data( | ||
context.bundle_analysis_comparison | ||
) | ||
if warning_threshold.type == "absolute": | ||
warning_threshold_readable = bytes_readable(warning_threshold.threshold) | ||
else: | ||
warning_threshold_readable = str(round(warning_threshold.threshold)) + "%" | ||
context = BundleCommentTemplateContext( | ||
has_cached=any(row["is_cached"] for row in bundle_rows), | ||
bundle_rows=bundle_rows, | ||
bundle_route_data=bundle_route_data, | ||
pull_url=get_bundle_analysis_pull_url(pull=context.pull.database_pull), | ||
total_size_delta=total_size_delta, | ||
status_level=context.commit_status_level.name, | ||
|
@@ -159,7 +172,7 @@ def _create_bundle_rows( | |
|
||
change_size = bundle_change.size_delta | ||
if change_size == 0: | ||
# Don't include bundles that were not changes in the table | ||
# Don't include bundles that were not changed in the table | ||
continue | ||
icon = "" | ||
if change_size > 0: | ||
|
@@ -184,3 +197,46 @@ def _create_bundle_rows( | |
) | ||
|
||
return bundle_rows | ||
|
||
def _create_bundle_route_data( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would we use this computation/stats elsewhere in the code, like in GQL for example? In case it makes sense for this to live in Shared There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not really, these computations are really for PR comments as it figures out the icons to use and how to display the size (eg bytes, MB, etc). On the app side these are taken care of by gazebo |
||
self, | ||
comparison: BundleAnalysisComparison, | ||
) -> dict[str, list[BundleRouteRow]]: | ||
""" | ||
Translate BundleRouteComparison dict data into a template compatible dict data | ||
""" | ||
bundle_route_data = {} | ||
changes_dict = comparison.bundle_routes_changes() | ||
|
||
for bundle_name, route_changes in changes_dict.items(): | ||
rows = [] | ||
for route_change in route_changes: | ||
change_size, icon = route_change.size_delta, "" | ||
size = ( | ||
"(removed)" | ||
if route_change.change_type == RouteChange.ChangeType.REMOVED | ||
else bytes_readable(route_change.size_head) | ||
) | ||
|
||
if change_size == 0: | ||
# Don't include bundles that were not changes in the table | ||
continue | ||
elif change_size > 0: | ||
icon = ":arrow_up:" | ||
elif change_size < 0: | ||
icon = ":arrow_down:" | ||
|
||
rows.append( | ||
BundleRouteRow( | ||
route_name=route_change.route_name, | ||
change_size_readable=bytes_readable(change_size), | ||
percentage_change_readable=f"{route_change.percentage_delta}%", | ||
change_icon=icon, | ||
route_size=size, | ||
) | ||
) | ||
|
||
# Only include bundles that have routes | ||
if rows: | ||
bundle_route_data[bundle_name] = rows | ||
return bundle_route_data |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
{% for bundle_name, routes in bundle_route_data.items %} | ||
<details> | ||
<summary>View changes by path for bundle: {{ bundle_name }}</summary> | ||
|
||
| File path | Size | Change | | ||
| --------- | ---- | ------ |{% for bundle_route_row in routes %} | ||
| {{bundle_route_row.route_name}} | {{bundle_route_row.route_size}} | {{bundle_route_row.change_size_readable}} ({{bundle_route_row.percentage_change_readable}}) {{bundle_route_row.change_icon}}{{bundle_route_row.}} |{% endfor %} | ||
|
||
</details> | ||
{% endfor %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be updated once shared PR is merged.