fitzee commented on code in PR #44336:
URL: https://github.com/apache/superset/pull/44336#discussion_r4060435452
##########
superset/commands/report/execute.py:
##########
@@ -142,26 +150,49 @@ def resolve_executor_user(model: ReportSchedule) ->
tuple["User", str]:
def _should_build_execution_context(model: ReportSchedule) -> bool:
- """
- Whether an execution should run under a :class:`ReportExecutionContext`.
-
- Reports always do — their behavior is unchanged. Alerts join them only when
- they deliver a rendered PNG/PDF screenshot to recipients, which happens
when
- ``ALERTS_ATTACH_REPORTS`` is enabled. Delivered screenshots must fail
closed:
- the context selects the fail-closed readiness predicate and disables
- partial-tile fallback, so a blank or incomplete capture raises instead of
- being delivered.
-
- CSV/text alerts, alerts without the attach flag, the non-delivered
- query-context capture, and UI thumbnails are deliberately excluded and keep
- their lenient capture contract.
- """
+ """Give every scheduled report and alert a shared deadline and ownership
context."""
+ return model.type in (ReportScheduleType.REPORT, ReportScheduleType.ALERT)
+
+
+def _uses_report_capture_contract(model: ReportSchedule) -> bool:
+ """Keep ownership separate from the existing rendered-alert capture
policy."""
+ return model.type == ReportScheduleType.REPORT or (
+ model.report_format in (ReportDataFormat.PNG, ReportDataFormat.PDF)
+ and feature_flag_manager.is_feature_enabled("ALERTS_ATTACH_REPORTS")
+ )
+
+
+def _execution_budget_seconds(model: ReportSchedule) -> float:
+ """Preserve alert Celery limits rather than applying the global report
budget."""
if model.type == ReportScheduleType.REPORT:
- return True
- return model.report_format in (
- ReportDataFormat.PNG,
- ReportDataFormat.PDF,
- ) and feature_flag_manager.is_feature_enabled("ALERTS_ATTACH_REPORTS")
+ return resolve_report_execution_budget_seconds(
+ app.config, working_timeout=model.working_timeout
+ )
+ return float(
+ get_report_task_timeout_options(
+ is_report=False, working_timeout=model.working_timeout,
config=app.config
+ ).get("soft_time_limit", float("inf"))
+ )
Review Comment:
Fixed in 3cbf3544bfead0a5fd3ce68b48c3757ed22db66a. `_phase_timeout()`
converts an unlimited deadline to `None` before it reaches the HTTP transport;
finite limits remain unchanged. Added real local-HTTP tests for CSV/XLSX GET
and POST exports and embedded-text alerts, covering both disabled worker limits
and an absent working timeout. All 14 new regression cases pass.
--
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]
