Copilot commented on code in PR #39973:
URL: https://github.com/apache/superset/pull/39973#discussion_r3236404529
##########
superset/commands/report/execute.py:
##########
@@ -244,10 +265,7 @@ def _get_url(
) and feature_flag_manager.is_feature_enabled("ALERT_REPORT_TABS"):
return self._get_tab_url(dashboard_state,
user_friendly=user_friendly)
- dashboard = self._report_schedule.dashboard
- dashboard_id_or_slug = (
- dashboard.uuid if dashboard and dashboard.uuid else dashboard.id
- )
+ dashboard_id_or_slug = dashboard.uuid or dashboard.id
Review Comment:
The same crash pattern that motivated this PR (`dashboard.uuid if dashboard
and dashboard.uuid else dashboard.id`) still exists in the sibling method
`get_dashboard_urls` further down in this file. Since `get_dashboard_urls` is
invoked from `_get_screenshots` without going through `_get_url`, a
missing/soft-deleted dashboard there will still raise `AttributeError:
'NoneType' object has no attribute 'id'`. Consider applying the same null-guard
(or extracting a shared helper that both methods call) so the fix actually
covers all dashboard URL resolution paths.
##########
superset/commands/report/execute.py:
##########
@@ -244,10 +265,7 @@ def _get_url(
) and feature_flag_manager.is_feature_enabled("ALERT_REPORT_TABS"):
return self._get_tab_url(dashboard_state,
user_friendly=user_friendly)
- dashboard = self._report_schedule.dashboard
- dashboard_id_or_slug = (
- dashboard.uuid if dashboard and dashboard.uuid else dashboard.id
- )
+ dashboard_id_or_slug = dashboard.uuid or dashboard.id
Review Comment:
This is a subtle behavior change from the prior expression. The previous
code only fell back to `dashboard.id` when `dashboard.uuid` was missing; with
`dashboard.uuid or dashboard.id`, any falsy `uuid` value (e.g. a UUID equal to
all zeros, which is falsy via `__bool__` on some representations, or an empty
string if the column type ever returns one) will silently fall through to `id`.
If `uuid` is always a `uuid.UUID` instance this is equivalent in practice, but
it's worth confirming — an explicit `dashboard.uuid if dashboard.uuid is not
None else dashboard.id` preserves the original intent more precisely.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
