Skip to content

Commit

Permalink
use None instead of empty string for missing digests and thumbs
Browse files Browse the repository at this point in the history
  • Loading branch information
villebro committed Jan 22, 2025
1 parent 7c7d60c commit 39312f7
Show file tree
Hide file tree
Showing 10 changed files with 29 additions and 25 deletions.
2 changes: 1 addition & 1 deletion superset-frontend/src/components/ListViewCard/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ interface CardProps {
subtitle?: ReactNode;
url?: string;
linkComponent?: ComponentType<LinkProps>;
imgURL?: string;
imgURL?: string | null;
imgFallbackURL?: string;
imgPosition?: BackgroundPosition;
description: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ const AddSliceCard: FC<{
lastModified?: string;
sliceName: string;
style?: CSSProperties;
thumbnailUrl?: string;
thumbnailUrl?: string | null;
visType: string;
}> = ({
datasourceUrl,
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/types/Dashboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export interface Dashboard {
slug?: string | null;
url: string;
dashboard_title: string;
thumbnail_url: string;
thumbnail_url: string | null;
published: boolean;
css?: string | null;
json_metadata?: string | null;
Expand Down
1 change: 1 addition & 0 deletions superset/charts/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ class ChartEntityResponseSchema(Schema):
certification_details = fields.String(
metadata={"description": certification_details_description}
)
thumbnail_url = fields.String(allow_none=True)


class ChartPostSchema(Schema):
Expand Down
2 changes: 1 addition & 1 deletion superset/dashboards/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ class DashboardGetResponseSchema(Schema):
dashboard_title = fields.String(
metadata={"description": dashboard_title_description}
)
thumbnail_url = fields.String()
thumbnail_url = fields.String(allow_none=True)
published = fields.Boolean()
css = fields.String(metadata={"description": css_description})
json_metadata = fields.String(metadata={"description": json_metadata_description})
Expand Down
6 changes: 3 additions & 3 deletions superset/models/dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,19 +225,19 @@ def dashboard_link(self) -> Markup:
return Markup(f'<a href="{self.url}">{title}</a>')

@property
def digest(self) -> str:
def digest(self) -> str | None:
return get_dashboard_digest(self)

@property
def thumbnail_url(self) -> str:
def thumbnail_url(self) -> str | None:
"""
Returns a thumbnail URL with a HEX digest. We want to avoid browser cache
if the dashboard has changed
"""
if digest := self.digest:
return f"/api/v1/dashboard/{self.id}/thumbnail/{digest}/"

Check warning on line 238 in superset/models/dashboard.py

View check run for this annotation

Codecov / codecov/patch

superset/models/dashboard.py#L237-L238

Added lines #L237 - L238 were not covered by tests

return ""
return None

Check warning on line 240 in superset/models/dashboard.py

View check run for this annotation

Codecov / codecov/patch

superset/models/dashboard.py#L240

Added line #L240 was not covered by tests

@property
def changed_by_name(self) -> str:
Expand Down
6 changes: 3 additions & 3 deletions superset/models/slice.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,19 +247,19 @@ def data(self) -> dict[str, Any]:
}

@property
def digest(self) -> str:
def digest(self) -> str | None:
return get_chart_digest(self)

@property
def thumbnail_url(self) -> str:
def thumbnail_url(self) -> str | None:
"""
Returns a thumbnail URL with a HEX digest. We want to avoid browser cache
if the dashboard has changed
"""
if digest := self.digest:
return f"/api/v1/chart/{self.id}/thumbnail/{digest}/"

Check warning on line 260 in superset/models/slice.py

View check run for this annotation

Codecov / codecov/patch

superset/models/slice.py#L259-L260

Added lines #L259 - L260 were not covered by tests

return ""
return None

Check warning on line 262 in superset/models/slice.py

View check run for this annotation

Codecov / codecov/patch

superset/models/slice.py#L262

Added line #L262 was not covered by tests

@property
def json_data(self) -> str:
Expand Down
16 changes: 8 additions & 8 deletions superset/thumbnails/digest.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,17 +90,16 @@ def _adjust_string_with_rls(
return unique_string


def get_dashboard_digest(dashboard: Dashboard) -> str:
def get_dashboard_digest(dashboard: Dashboard) -> str | None:
config = current_app.config
datasources = dashboard.datasources
try:
executor_type, executor = get_executor(
executors=config["THUMBNAIL_EXECUTORS"],
model=dashboard,
current_user=get_current_user(),
)
except ExecutorNotFoundError:
return ""
return None

if func := config["THUMBNAIL_DASHBOARD_DIGEST_FUNC"]:
return func(dashboard, executor_type, executor)
Expand All @@ -111,28 +110,29 @@ def get_dashboard_digest(dashboard: Dashboard) -> str:
)

unique_string = _adjust_string_for_executor(unique_string, executor_type, executor)
unique_string = _adjust_string_with_rls(unique_string, datasources, executor)
unique_string = _adjust_string_with_rls(
unique_string, dashboard.datasources, executor
)

return md5_sha_from_str(unique_string)


def get_chart_digest(chart: Slice) -> str:
def get_chart_digest(chart: Slice) -> str | None:
config = current_app.config
datasource = chart.datasource
try:
executor_type, executor = get_executor(
executors=config["THUMBNAIL_EXECUTORS"],
model=chart,
current_user=get_current_user(),
)
except ExecutorNotFoundError:
return ""
return None

if func := config["THUMBNAIL_CHART_DIGEST_FUNC"]:
return func(chart, executor_type, executor)

unique_string = f"{chart.params or ''}.{executor}"
unique_string = _adjust_string_for_executor(unique_string, executor_type, executor)
unique_string = _adjust_string_with_rls(unique_string, [datasource], executor)
unique_string = _adjust_string_with_rls(unique_string, [chart.datasource], executor)

return md5_sha_from_str(unique_string)
13 changes: 8 additions & 5 deletions superset/utils/screenshots.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,18 @@

class BaseScreenshot:
driver_type = current_app.config["WEBDRIVER_TYPE"]
url: str
digest: str | None
screenshot: bytes | None
thumbnail_type: str = ""
element: str = ""
window_size: WindowSize = DEFAULT_SCREENSHOT_WINDOW_SIZE
thumb_size: WindowSize = DEFAULT_SCREENSHOT_THUMBNAIL_SIZE

def __init__(self, url: str, digest: str):
self.digest: str = digest
def __init__(self, url: str, digest: str | None):
self.digest = digest

Check warning on line 68 in superset/utils/screenshots.py

View check run for this annotation

Codecov / codecov/patch

superset/utils/screenshots.py#L68

Added line #L68 was not covered by tests
self.url = url
self.screenshot: bytes | None = None
self.screenshot = None

Check warning on line 70 in superset/utils/screenshots.py

View check run for this annotation

Codecov / codecov/patch

superset/utils/screenshots.py#L70

Added line #L70 was not covered by tests

def driver(self, window_size: WindowSize | None = None) -> WebDriver:
window_size = window_size or self.window_size
Expand Down Expand Up @@ -227,7 +230,7 @@ class ChartScreenshot(BaseScreenshot):
def __init__(
self,
url: str,
digest: str,
digest: str | None,
window_size: WindowSize | None = None,
thumb_size: WindowSize | None = None,
):
Expand All @@ -248,7 +251,7 @@ class DashboardScreenshot(BaseScreenshot):
def __init__(
self,
url: str,
digest: str,
digest: str | None,
window_size: WindowSize | None = None,
thumb_size: WindowSize | None = None,
):
Expand Down
4 changes: 2 additions & 2 deletions tests/unit_tests/thumbnails/test_digest.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ def prepare_datasource_mock(
False,
False,
[],
"",
None,
),
(
None,
Expand Down Expand Up @@ -353,7 +353,7 @@ def test_dashboard_digest(
False,
False,
None,
"",
None,
),
(
None,
Expand Down

0 comments on commit 39312f7

Please sign in to comment.