codeant-ai-for-open-source[bot] commented on code in PR #42624:
URL: https://github.com/apache/superset/pull/42624#discussion_r3694423819
##########
superset/utils/webdriver.py:
##########
@@ -380,11 +440,15 @@ def _wait_for_charts_ready(
elapsed,
context_suffix,
)
- readiness_predicate = (
- CHART_CONTAINER_READY_JS
- if element_name == "chart-container"
- else CHART_HOLDERS_READY_JS
- )
+ if element_name == "chart-container":
+ readiness_predicate = CHART_CONTAINER_READY_JS
+ elif report_execution_context:
+ readiness_predicate = REPORT_CHART_HOLDERS_READY_JS
Review Comment:
✅ **Customized review instruction saved!**
**Instruction:**
> Do not require below-the-fold chart holders for the standard screenshot
readiness predicate, since virtualized dashboards may never render off-screen
holders; rely on the tiled capture path for full-dashboard readiness.
**Applied to:**
- `superset/utils/webdriver.py`
---
💡 *To manage or update this instruction, visit: [CodeAnt AI
Settings](https://app.codeant.ai/org/settings/learnings)*
##########
superset/tasks/scheduler.py:
##########
@@ -98,19 +100,14 @@ def scheduler(self: Task) -> None: # pylint:
disable=unused-argument
triggered_at, active_schedule.crontab, active_schedule.timezone
):
logger.info("Scheduling alert %s eta: %s", active_schedule.name,
schedule)
- async_options = {"eta": schedule}
- if (
- active_schedule.working_timeout is not None
- and current_app.config["ALERT_REPORTS_WORKING_TIME_OUT_KILL"]
- ):
- async_options["time_limit"] = (
- active_schedule.working_timeout
- + current_app.config["ALERT_REPORTS_WORKING_TIME_OUT_LAG"]
- )
- async_options["soft_time_limit"] = (
- active_schedule.working_timeout
- +
current_app.config["ALERT_REPORTS_WORKING_SOFT_TIME_OUT_LAG"]
- )
+ async_options = {
+ "eta": schedule,
+ **get_report_task_timeout_options(
+ is_report=active_schedule.type ==
ReportScheduleType.REPORT,
+ working_timeout=active_schedule.working_timeout,
+ config=current_app.config,
+ ),
+ }
execute.apply_async((active_schedule.id,), **async_options)
Review Comment:
✅ **Customized review instruction saved!**
**Instruction:**
> Do not flag the resolver-floor behavior in report execution timeout
calculations; it is intentional, documented, and consistently applied to both
Celery limits and in-process deadlines.
**Applied to:**
- `superset/tasks/scheduler.py`
---
💡 *To manage or update this instruction, visit: [CodeAnt AI
Settings](https://app.codeant.ai/org/settings/learnings)*
##########
superset/utils/screenshot_utils.py:
##########
@@ -406,16 +515,12 @@ def _raise_if_budget_exhausted(elapsed: float,
remaining_budget: float) -> None:
# Wait for scroll to settle and content to load
page.wait_for_timeout(SCROLL_SETTLE_TIMEOUT_MS)
- # Recompute the remaining budget after the scroll-settle sleep --
- # which itself consumes real wall-clock time -- rather than
- # reusing the value from before it, so the readiness-check
- # timeout below is capped against a fresh number instead of a
- # stale one that would let each tile overrun the budget by up
- # to one settle interval.
- tile_wait_start = time.monotonic()
- elapsed = tile_wait_start - start_time
- remaining_budget = wait_budget_seconds - elapsed
- _raise_if_budget_exhausted(elapsed, remaining_budget)
+ # Re-check after the scroll-settle sleep -- which itself consumes
+ # real wall-clock time -- so the readiness-check timeout below is
+ # derived from a fresh remaining value instead of a stale one
+ # that would let each tile overrun the budget by up to one settle
+ # interval (_timeout_seconds also recomputes at call time).
+ _raise_if_budget_exhausted()
Review Comment:
✅ **Customized review instruction saved!**
**Instruction:**
> Do not require capping the scroll-settle sleep when budget checks occur
immediately before scrolling and after the sleep, since the bounded overshoot
is intentionally covered by the cleanup reserve and hard grace period.
**Applied to:**
- `superset/utils/screenshot_utils.py`
---
💡 *To manage or update this instruction, visit: [CodeAnt AI
Settings](https://app.codeant.ai/org/settings/learnings)*
##########
superset/commands/report/execute.py:
##########
@@ -1472,11 +1808,62 @@ def __init__(self, task_id: str, model_id: int,
scheduled_dttm: datetime):
self._execution_id = UUID(task_id)
def run(self) -> None:
+ monotonic_started_at = time.monotonic()
+ report_execution_context: ReportExecutionContext | None = None
+ owns_report_working_state = False
try:
self.validate()
if not self._model:
raise ReportScheduleExecuteUnexpectedError()
+ if self._model.type == ReportScheduleType.REPORT:
+ # An invocation that enters on WORKING is a duplicate or stale
+ # recovery, not the owner that created the active row. Its
state
+ # handler may terminalize a stale execution, but the command
+ # boundary must never infer ownership from a replayed UUID.
+ owns_report_working_state = (
+ self._model.last_state != ReportState.WORKING
+ )
+ total_seconds = resolve_report_execution_budget_seconds(
+ app.config,
+ working_timeout=self._model.working_timeout,
+ )
Review Comment:
✅ **Customized review instruction saved!**
**Instruction:**
> Preserve the minimum execution-budget floor when a configured
working_timeout is below the capture, delivery, and cleanup reserves, and emit
a runtime warning when the floor is applied; do not require strict adherence to
sub-viable timeouts.
**Applied to:**
- `superset/commands/report/execute.py`
---
💡 *To manage or update this instruction, visit: [CodeAnt AI
Settings](https://app.codeant.ai/org/settings/learnings)*
--
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]
