You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "eschutho (via GitHub)" <gi...@apache.org> on 2026/07/16 22:16:18 UTC
[PR] fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill [superset]
eschutho opened a new pull request, #42118:
URL: https://github.com/apache/superset/pull/42118
### Decisions made that were not in the instructions
- **Safety margin: 300s.** Budget = 1740s (observed Celery hard `task_time_limit` for report execution) minus a 300s margin for the pipeline steps that run after tiling: combining tiles, building the PDF, and uploading/delivering the notification. Resulting budget constant: `TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS = 1440`.
- **Config-derivation approach: fixed constant, not runtime-derived.** Investigated whether the actual Celery time limit is reachable from `take_tiled_screenshot()`. It is not reliably reachable: `superset/tasks/scheduler.py` only sets `time_limit`/`soft_time_limit` per report schedule via `apply_async(...)`, and only when that schedule defines `working_timeout` (gated by `ALERT_REPORTS_WORKING_TIME_OUT_KILL`) — there's no static Superset config key that always reflects the limit actually enforced at runtime. This function can also run outside a Celery task entirely (e.g. synchronous thumbnail generation), where there is no task context to introspect at all. Given that, I used a conservative module-level constant with a comment documenting the 1740s constraint it must stay under, per the task instructions' fallback guidance.
- **Animation wait exhaustion doesn't raise.** When the budget runs out mid-tile for the *spinner* wait, we abort (raise) because that's the actual readiness check. When it runs out for the trailing *animation* wait (cosmetic settling after the spinner already cleared), we just skip that wait rather than raising, since the tile has already been verified ready.
### SUMMARY
`take_tiled_screenshot()` in `superset/utils/screenshot_utils.py` waited up to `load_wait` (production: `SCREENSHOT_LOAD_WAIT`, up to 600s) per tile for loading spinners to clear, with no cap on cumulative wait across all tiles. On dashboards with many tiles, `load_wait × num_tiles` could exceed the Celery `task_time_limit`, so the worker was SIGKILLed mid-capture instead of the report failing cleanly — observed in production on 2026-07-13 as `Tiled screenshot failed: SoftTimeLimitExceeded()`.
This PR:
- Tracks wall-clock elapsed time across the whole tiled-screenshot operation via `time.monotonic()`.
- Caps each tile's spinner wait (and trailing animation wait) at whatever remains of a fixed `TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS = 1440` (1740s Celery limit − 300s margin) budget, so a slow dashboard degrades gracefully instead of accumulating unbounded waits.
- If the budget is exhausted before a tile's readiness can even be checked, logs an `ERROR` with tiles completed/remaining, elapsed time, and budget, then raises a new `TiledScreenshotBudgetExceededError` — a hard failure, not a silent `None`/partial result. This propagates up through `WebDriverPlaywright.get_screenshot` → `ChartScreenshot.get_screenshot` → `AsyncExecuteReportScheduleCommand._get_screenshots`, which already wraps it in `ReportScheduleScreenshotFailedError`, so the report errors and owners are notified instead of receiving a blank/partial report.
### NOT changed (intentionally)
- The per-tile readiness predicate itself (the `.loading` viewport-spinner check) and its timeout/warn-and-continue semantics on an individual `PlaywrightTimeout` are unchanged — that's owned by the concurrent `fix-tile-readiness-check` PR. These two PRs touch adjacent lines in the same loop; whichever merges second will need to rebase/resolve.
- No changes to `superset/utils/webdriver.py`'s existing fallback-to-standard-screenshot behavior when `take_tiled_screenshot()` returns `None` for non-budget errors (that path is untouched).
### TESTING INSTRUCTIONS
- New unit tests in `tests/unit_tests/utils/test_screenshot_utils.py` (class `TestTileWaitBudget`):
- `test_per_tile_wait_shrinks_as_budget_depletes` — mocks `time.monotonic` to simulate elapsing time and asserts `wait_for_function` timeouts shrink tile-over-tile as the budget depletes.
- `test_budget_exhausted_raises_and_stops_capturing` — simulates the budget hitting zero mid-loop; asserts `TiledScreenshotBudgetExceededError` is raised, no further tiles are captured or combined, and an `ERROR` is logged with the tiles-captured/total/elapsed/budget values.
- `test_fast_dashboard_matches_default_behavior` — a fast (real-time) run stays well under budget and behaves identically to before (full `load_wait`/`animation_wait` per tile).
- Ran the full existing `tests/unit_tests/utils/test_screenshot_utils.py` suite: 23 passed, 1 pre-existing failure (`test_per_tile_spinner_timeout_logs_warning_and_continues`) confirmed unrelated — it fails identically on `master` in this environment due to a local Playwright version mismatch (`PlaywrightTimeout()` requiring a `message` arg), not this change.
- Ran `ruff check`, `ruff format --check`, and `mypy` on the changed files — clean aside from 3 pre-existing mypy errors in this file that are unchanged from `master`.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill [superset]
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho merged PR #42118:
URL: https://github.com/apache/superset/pull/42118
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42118:
URL: https://github.com/apache/superset/pull/42118#issuecomment-5065345044
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42118:
URL: https://github.com/apache/superset/pull/42118#issuecomment-5147390591
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42118:
URL: https://github.com/apache/superset/pull/42118#issuecomment-4998126660
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #42118:
URL: https://github.com/apache/superset/pull/42118#discussion_r3635509152
##########
tests/unit_tests/utils/test_screenshot_utils.py:
##########
@@ -568,3 +576,279 @@ def test_animation_wait_default_is_zero(self):
sig = inspect.signature(take_tiled_screenshot)
assert sig.parameters["animation_wait"].default == 0
+
+
+class TestTileWaitBudget:
+ @pytest.fixture
+ def mock_page(self):
+ """Create a mock Playwright page object for a 3-tile (5000px) dashboard."""
+ page = MagicMock()
+ element = MagicMock()
+ page.locator.return_value = element
+ page.evaluate.return_value = {
+ "height": 5000,
+ "top": 100,
+ "left": 50,
+ "width": 800,
+ }
+ page.screenshot.return_value = b"fake_screenshot_data"
+ return page
+
+ class _FakeClock:
+ """Stateful monotonic() stand-in the test advances explicitly.
+
+ Robust to how many times the code under test samples the clock per
+ tile (budget check, per-tile wait timing, animation budget) -- only
+ explicit advances move time forward.
+ """
+
+ def __init__(self) -> None:
+ self.now = 0.0
+
+ def __call__(self) -> float:
+ return self.now
+
+ def test_per_tile_wait_shrinks_as_budget_depletes(self, mock_page, monkeypatch):
+ """Each tile's readiness-wait timeout is capped at the remaining budget."""
+ monkeypatch.setattr(
+ "superset.utils.screenshot_utils.TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS", # noqa: E501
+ 1000,
+ )
+ clock = self._FakeClock()
+ # Simulate slow tiles: the readiness wait itself consumes wall time,
+ # so each subsequent tile sees less remaining budget.
+ wait_durations = iter([950, 40, 5])
+
+ def slow_wait(*args, **kwargs):
+ clock.now += next(wait_durations)
+
+ mock_page.wait_for_function.side_effect = slow_wait
+
+ with patch("superset.utils.screenshot_utils.time.monotonic", new=clock):
+ with patch("superset.utils.screenshot_utils.combine_screenshot_tiles"):
+ result = take_tiled_screenshot(
+ mock_page, "dashboard", tile_height=2000, load_wait=100
+ )
+
+ assert result is not None
+ timeouts = [
+ call[1]["timeout"] for call in mock_page.wait_for_function.call_args_list
+ ]
+ # remaining budget at each tile's wait: 1000, 50, 10 seconds
+ # -> capped timeouts shrink
+ assert timeouts == [100 * 1000, 50 * 1000, 10 * 1000]
+ assert timeouts == sorted(timeouts, reverse=True)
+
+ def test_budget_exhausted_raises_and_stops_capturing(self, mock_page, monkeypatch):
+ """Exhausting the budget aborts cleanly instead of capturing unchecked."""
+ monkeypatch.setattr(
+ "superset.utils.screenshot_utils.TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS", # noqa: E501
+ 1000,
+ )
+ clock = self._FakeClock()
+ # Tile 0's readiness wait consumes the whole budget; tile 1's budget
+ # check then sees remaining <= 0 and raises before capturing.
+ mock_page.wait_for_function.side_effect = lambda *args, **kwargs: setattr(
+ clock, "now", 1000.0
+ )
+
+ with patch("superset.utils.screenshot_utils.time.monotonic", new=clock):
+ with patch(
+ "superset.utils.screenshot_utils.combine_screenshot_tiles"
+ ) as mock_combine:
+ with patch("superset.utils.screenshot_utils.logger") as mock_logger:
+ with pytest.raises(TiledScreenshotBudgetExceededError):
+ take_tiled_screenshot(
+ mock_page, "dashboard", tile_height=2000, load_wait=100
+ )
+
+ # Only the first tile was captured before the budget ran out.
+ assert mock_page.screenshot.call_count == 1
+ # Tiles were never combined -- the function raised before that point.
+ mock_combine.assert_not_called()
+
+ # Budget exhaustion is a customer chart-loading issue, not a Superset
+ # system fault, so it must log at WARNING (not ERROR) -- consistent
+ # with the #38130/#38441 precedent for screenshot timeout logging.
+ assert mock_logger.error.call_count == 0
+ mock_logger.warning.assert_called_once()
+ warning_args = mock_logger.warning.call_args[0]
+ assert "budget exhausted" in warning_args[0]
+ # tile index, tiles total, tiles captured, tiles total,
+ # elapsed seconds, budget seconds, log-context suffix
+ assert warning_args[1] == 2
+ assert warning_args[2] == 3
+ assert warning_args[3] == 1
+ assert warning_args[4] == 3
+ assert warning_args[5] == 1000
+ assert warning_args[6] == 1000
+ assert warning_args[7] == ""
+
+ def test_budget_exhausted_warning_includes_log_context(
+ self, mock_page, monkeypatch
+ ):
+ """log_context (e.g. report execution id) is appended to the warning."""
+ monkeypatch.setattr(
+ "superset.utils.screenshot_utils.TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS", # noqa: E501
+ 1000,
+ )
+ clock = self._FakeClock()
+ # Tile 0's readiness wait consumes the whole budget; tile 1's budget
+ # check then sees remaining <= 0 and raises.
+ mock_page.wait_for_function.side_effect = lambda *args, **kwargs: setattr(
+ clock, "now", 1000.0
+ )
+ with patch("superset.utils.screenshot_utils.time.monotonic", new=clock):
+ with patch("superset.utils.screenshot_utils.combine_screenshot_tiles"):
+ with patch("superset.utils.screenshot_utils.logger") as mock_logger:
+ with pytest.raises(TiledScreenshotBudgetExceededError):
+ take_tiled_screenshot(
+ mock_page,
+ "dashboard",
+ tile_height=2000,
+ load_wait=100,
+ log_context="execution_id=abc-123",
+ )
+
+ warning_args = mock_logger.warning.call_args[0]
+ assert warning_args[-1] == " [execution_id=abc-123]"
+
+ def test_per_tile_timing_debug_line_logged(self, mock_page):
+ """Each tile logs a DEBUG timing breakdown (spinner wait, animation wait)."""
+ with patch("superset.utils.screenshot_utils.logger") as mock_logger:
+ with patch("superset.utils.screenshot_utils.combine_screenshot_tiles"):
+ take_tiled_screenshot(
+ mock_page,
+ "dashboard",
+ tile_height=2000,
+ log_context="cache_key=xyz",
+ )
+
+ timing_calls = [
+ call for call in mock_logger.debug.call_args_list if "timing" in call[0][0]
+ ]
+ assert len(timing_calls) == 3
+ for i, call in enumerate(timing_calls):
+ args = call[0]
+ assert args[1] == i + 1 # tile index
+ assert args[2] == 3 # total tiles
+ assert args[-1] == " [cache_key=xyz]"
+
+ def test_fast_dashboard_matches_default_behavior(self, mock_page):
+ """Well under budget, waits are not capped and behavior is unchanged."""
+ with patch("superset.utils.screenshot_utils.combine_screenshot_tiles"):
+ result = take_tiled_screenshot(
+ mock_page,
+ "dashboard",
+ tile_height=2000,
+ load_wait=30,
+ animation_wait=5,
+ )
+
+ assert result is not None
+ assert mock_page.screenshot.call_count == 3
+
+ for call in mock_page.wait_for_function.call_args_list:
+ assert call[1]["timeout"] == 30 * 1000
+
+ animation_calls = [
+ call
+ for call in mock_page.wait_for_timeout.call_args_list
+ if call[0][0] == 5 * 1000
+ ]
+ assert len(animation_calls) == 3
+
+
+class TestResolveWaitBudgetSeconds:
+ """The budget is derived from the running Celery task's own time limit
+ when available, and falls back to the fixed constant otherwise."""
+
+ def _mock_task(self, soft=None, hard=None):
+ task = MagicMock()
+ task.request.timelimit = (soft, hard)
+ return task
+
+ def test_derives_budget_from_soft_time_limit(self):
+ """soft_time_limit is preferred over the hard time_limit when both are set."""
+ task = self._mock_task(soft=90, hard=120)
+ with patch("superset.utils.screenshot_utils.current_task", task):
+ budget = _resolve_wait_budget_seconds()
+
+ # margin = min(300, 90 * 0.2) = 18; budget = 90 - 18 = 72
+ assert budget == 72
+
+ def test_small_task_limit_yields_positive_scaled_margin_budget(self):
+ """A 120s thumbnail-task limit (superset-shell#4389) still gets a
+ usable, positive budget via the scaled-down margin, not the fixed
+ 300s margin that would otherwise wipe it out."""
+ task = self._mock_task(soft=None, hard=120)
+ with patch("superset.utils.screenshot_utils.current_task", task):
+ budget = _resolve_wait_budget_seconds()
+
+ # margin = min(300, 120 * 0.2) = 24; budget = 120 - 24 = 96
+ assert budget == 96
+ assert budget > 0
+ assert budget < 120
+
+ def test_no_task_context_falls_back_to_constant(self):
+ """Outside of a Celery task, the fixed fallback budget is used."""
+ with patch("superset.utils.screenshot_utils.current_task", None):
+ budget = _resolve_wait_budget_seconds()
+
+ assert budget == TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS
+
+ def test_task_with_no_timelimit_falls_back_to_constant(self):
+ """A task with no soft or hard limit set falls back to the constant."""
+ task = self._mock_task(soft=None, hard=None)
+ with patch("superset.utils.screenshot_utils.current_task", task):
+ budget = _resolve_wait_budget_seconds()
+
+ assert budget == TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS
+
+ def test_derivation_exception_falls_back_to_constant(self):
+ """Any failure while inspecting the task context must never break a
+ screenshot -- fall back to the constant and log at DEBUG."""
+
+ class _BrokenTask:
+ """Simulates a task-like object whose .request raises."""
+
+ @property
+ def request(self):
+ raise RuntimeError("boom")
+
+ with patch("superset.utils.screenshot_utils.current_task", _BrokenTask()):
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Exception uses string literal directly</b></div>
<div id="fix">
Exception raised with string literal directly instead of assigning to a variable first.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
def request(self):
error_msg = "boom"
raise RuntimeError(error_msg)
with patch("superset.utils.screenshot_utils.current_task", _BrokenTask()):
````
</div>
</details>
</div>
<small><i>Code Review Run #1c7538</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42118:
URL: https://github.com/apache/superset/pull/42118#issuecomment-4997110382
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill [superset]
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho commented on PR #42118:
URL: https://github.com/apache/superset/pull/42118#issuecomment-5147790724
Went through the two open CodeAnt findings against the current (reworked) revision:
**1. "Stale `remaining_budget` reused after the scroll-settle sleep" — already fixed; the comment references the previous revision.** Its line anchors point at code that no longer exists (e.g. `_resolve_wait_budget_seconds`, removed when this PR was reduced to reuse #42427's shared budget helper). The current revision recomputes elapsed/remaining *after* `page.wait_for_timeout(SCROLL_SETTLE_TIMEOUT_MS)` and re-checks exhaustion there before deriving `tile_load_wait` — see the "Recompute the remaining budget after the scroll-settle sleep" block in `take_tiled_screenshot`, pinned by `test_readiness_wait_uses_budget_recomputed_after_scroll_settle`. Resolving as fixed.
**2. "No budget re-check between the animation wait and `page.screenshot()`" — accurate as a control-flow observation, but intentional; declining the suggested change.** Reasoning:
- The animation wait is capped at `min(animation_wait, remaining_budget)`, so it can drain the budget to exactly zero but **cannot overshoot it** — and the budget is `task limit − cleanup margin` (`min(300s, 20%)`), so even at full consumption the entire margin remains for capture/combine/PDF/delivery, which is precisely what the margin exists to cover. The finding's claim that the function can return "despite exceeding the task limit" doesn't hold: no wait in this path can run past the budget, let alone the limit.
- The tile captured after that wait is **fully readiness-verified** — its capped readiness wait already completed within budget. `page.screenshot()` is a synchronous snapshot, not a wait. The budget-exhaustion raise exists to prevent unverifiable *future waits*, which doesn't apply here.
- The only behavioral effect of adding the suggested re-check would be on the **last tile**: a complete, verified, within-budget capture would be converted into a hard report failure at the finish line (mid-loop tiles are already covered by the next iteration's top-of-loop check, which raises before any work on the next tile). That trades a rare successful report for a failure with no reduction in SIGKILL risk.
- This is the documented design in the code: the animation wait is "cosmetic settling, not a readiness check, so we simply skip it (rather than raise) once the budget runs out."
Also in the latest push: refreshed a comment in the budget-exhaustion `except` clause that still described the pre-#42273 caller behavior ("callers treat `return None` as fall back to a standard, unchecked screenshot") — since #42273 merged, callers fail the capture loudly instead. No behavior change; CI re-running on the amended head.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill [superset]
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho commented on code in PR #42118:
URL: https://github.com/apache/superset/pull/42118#discussion_r3693769604
##########
superset/utils/screenshot_utils.py:
##########
@@ -286,6 +315,15 @@ def take_tiled_screenshot(
# match `except PlaywrightTimeout` and incorrectly propagate instead of
# degrading to `None` like every other unexpected error in this function.
readiness_timeout = False
+ # Cap the whole tiled operation against the running Celery task's own
+ # time limit, using the same runtime derivation as the non-tiled
+ # readiness wait (#42253/#42427). Unlike that path, a None budget does
+ # not mean "keep the configured timeout": per-tile waits accumulate, so
+ # the operation falls back to a fixed total ceiling instead.
+ wait_budget_seconds = resolve_screenshot_task_budget_seconds(log_context)
+ if wait_budget_seconds is None:
+ wait_budget_seconds = float(TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS)
+ start_time = time.monotonic()
Review Comment:
Good catch, and agreed it deserved both-paths-on-one-clock: implemented exactly as suggested (all three coordinated changes) in follow-up #42661, with tests pinning that pre-capture elapsed time now reduces the first tile's capped wait, and that the omitted-anchor default is unchanged.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #42118:
URL: https://github.com/apache/superset/pull/42118#discussion_r3599324397
##########
superset/utils/screenshot_utils.py:
##########
@@ -28,6 +29,35 @@
# Time to wait after scrolling for content to settle and load (in milliseconds)
SCROLL_SETTLE_TIMEOUT_MS = 1000
+# Total wall-clock budget, in seconds, for the entire tiled-screenshot
+# operation (element lookup plus all per-tile spinner/animation waits
+# combined). Each tile's wait is capped at whatever remains of this budget
+# so a slow dashboard degrades gracefully instead of running past the
+# Celery task time limit and getting SIGKILLed mid-capture.
+#
+# This is a conservative fixed constant rather than a value derived from
+# Celery's configured task time limit at runtime, because that limit isn't
+# reliably reachable from here: superset/tasks/scheduler.py sets it
+# per-report-schedule via `apply_async(time_limit=..., soft_time_limit=...)`
+# only when a schedule defines `working_timeout`
+# (ALERT_REPORTS_WORKING_TIME_OUT_KILL); there is no static config value
+# that always reflects the limit actually enforced, and this function can
+# also run outside of a Celery task entirely (e.g. synchronous thumbnail
+# generation).
+#
+# Production has observed a Celery hard task_time_limit of 1740s (29 min)
+# for report execution (2026-07-13 incident: a tiled screenshot was killed
+# mid-capture with SoftTimeLimitExceeded). This budget leaves a 300s margin
+# under that ceiling for the rest of the pipeline that runs after tiling
+# completes: combining tiles into one image, building the PDF, and
+# uploading/delivering the notification.
+TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS = 1440 # 1740s limit - 300s margin
Review Comment:
**Suggestion:** The hardcoded 1440-second budget is not aligned with per-schedule Celery `time_limit`/`soft_time_limit` overrides, which can be configured lower via `working_timeout`. In those deployments this function can still run longer than the actual worker limit and get killed before failing cleanly. Derive the budget from the active report/task timeout when available (or enforce a minimum of configured task limits), and only fall back to a static default when no runtime limit is accessible. [possible bug]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
❌ Report screenshot tasks can be killed mid-tiling.
⚠️ Budgeted waits ignore per-schedule Celery time limits.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a report schedule with a lower `working_timeout` via the `working_timeout`
field exposed in the Reports API (`superset/reports/api.py:148, 206`), e.g. 600 seconds.
2. Celery beat uses that `working_timeout` to set per-schedule `time_limit` and
`soft_time_limit` in `scheduler()` at `superset/tasks/scheduler.py:23-35`, so the worker
enforces a shorter execution window than the global 1740s default.
3. When the schedule fires, `reports.execute` (`superset/tasks/scheduler.py:38-56`) runs
`AsyncExecuteReportScheduleCommand.run()` (`superset/commands/report/execute.py:22-77`),
which calls `_get_screenshots()` (`superset/commands/report/execute.py:520-79`) and
ultimately `BaseScreenshot.get_screenshot()` (`superset/utils/screenshots.py:53-62`) using
`WebDriverPlaywright` and `take_tiled_screenshot()` for large dashboards.
4. `take_tiled_screenshot()` applies the fixed `TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS
= 1440` (`superset/utils/screenshot_utils.py:54, 61-83, 93-121`); on slow, many-tile
dashboards this allows up to ~1440s of tiling before `TiledScreenshotBudgetExceededError`,
but the Celery worker may hit its lower per-schedule `time_limit`/`soft_time_limit` first,
yielding `SoftTimeLimitExceeded` or a hard kill instead of the intended clean budget
exhaustion failure.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ed6d8ebe609b419da381394a77026170&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=ed6d8ebe609b419da381394a77026170&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/utils/screenshot_utils.py
**Line:** 54:54
**Comment:**
*Possible Bug: The hardcoded 1440-second budget is not aligned with per-schedule Celery `time_limit`/`soft_time_limit` overrides, which can be configured lower via `working_timeout`. In those deployments this function can still run longer than the actual worker limit and get killed before failing cleanly. Derive the budget from the active report/task timeout when available (or enforce a minimum of configured task limits), and only fall back to a static default when no runtime limit is accessible.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42118&comment_hash=5b9c742566caae0ed56f4248ef3e0ea5c585ee40b22d48ca08660042c8578a1b&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42118&comment_hash=5b9c742566caae0ed56f4248ef3e0ea5c585ee40b22d48ca08660042c8578a1b&reaction=dislike'>👎</a>
##########
superset/utils/screenshot_utils.py:
##########
@@ -150,9 +210,12 @@ def take_tiled_screenshot(
)
# Wait for scroll to settle and content to load
page.wait_for_timeout(SCROLL_SETTLE_TIMEOUT_MS)
- # Wait for any loading spinners visible in the current viewport to clear.
+ # Wait for any loading spinners visible in the current viewport to clear,
+ # capped at whatever remains of the total time budget so a slow
+ # dashboard degrades gracefully instead of exceeding it.
# Only check viewport-visible spinners to avoid blocking on
# virtualization placeholders rendered for off-screen charts.
+ tile_load_wait = min(load_wait, remaining_budget)
Review Comment:
**Suggestion:** The spinner timeout is derived from a stale remaining-budget value that was computed before the mandatory scroll-settle sleep. This lets each tile wait up to one extra settle interval beyond the declared global budget, so total runtime can still exceed the intended cap. Recompute the remaining budget immediately before `wait_for_function` (after settle wait) and cap spinner wait with that refreshed value. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
⚠️ Tiled screenshots may exceed configured global wait budget.
⚠️ Celery task margin eroded on heavily tiled dashboards.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Schedule a dashboard report so Celery runs `reports.execute` at
`superset/tasks/scheduler.py:38-56`, which invokes
`AsyncExecuteReportScheduleCommand.run()` at `superset/tasks/scheduler.py:131-135`.
2. Inside `AsyncExecuteReportScheduleCommand._get_screenshots()`
(`superset/commands/report/execute.py:520-79`), a `DashboardScreenshot` or
`ChartScreenshot` is constructed and `BaseScreenshot.get_screenshot()`
(`superset/utils/screenshots.py:53-62`) is called.
3. `BaseScreenshot.driver()` returns `WebDriverPlaywright` when enabled
(`superset/utils/screenshots.py:37-41`), whose `get_screenshot()` calls
`take_tiled_screenshot()` (`superset/utils/webdriver.py:40-51, 213-219, 399-49`) for large
dashboards.
4. In `take_tiled_screenshot()` (`superset/utils/screenshot_utils.py:61-121`),
`remaining_budget` is computed before `page.wait_for_timeout(SCROLL_SETTLE_TIMEOUT_MS)` at
line ~93, then `tile_load_wait = min(load_wait, remaining_budget)` at line ~99 uses that
stale value; this lets each tile spend `SCROLL_SETTLE_TIMEOUT_MS` extra per loop beyond
the nominal `TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS`, so total wall-clock time can
exceed the configured budget before `TiledScreenshotBudgetExceededError` is raised.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=0c858d2b97344bc5be87dfe5c5b082b3&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=0c858d2b97344bc5be87dfe5c5b082b3&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/utils/screenshot_utils.py
**Line:** 218:218
**Comment:**
*Logic Error: The spinner timeout is derived from a stale remaining-budget value that was computed before the mandatory scroll-settle sleep. This lets each tile wait up to one extra settle interval beyond the declared global budget, so total runtime can still exceed the intended cap. Recompute the remaining budget immediately before `wait_for_function` (after settle wait) and cap spinner wait with that refreshed value.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42118&comment_hash=7100d668f092031d049f284f9a9973ecd220d0cff9ff6b45c5ee616121ea3bc7&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42118&comment_hash=7100d668f092031d049f284f9a9973ecd220d0cff9ff6b45c5ee616121ea3bc7&reaction=dislike'>👎</a>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill [superset]
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho commented on PR #42118:
URL: https://github.com/apache/superset/pull/42118#issuecomment-5064287617
Thanks for the review, @rusackas — went through the bot threads:
- **CodeAnt's "stale `remaining_budget`" finding (Major, both threads)**: confirmed real. `remaining_budget` was computed once per tile before the scroll-settle sleep, then reused (stale) to cap the readiness wait after it, letting each tile overrun the budget by up to one settle interval. Fixed in bcb5957: recompute elapsed/remaining budget after the settle sleep (and re-check for exhaustion there too) before deriving the readiness timeout. Added a regression test (`test_readiness_wait_uses_budget_recomputed_after_scroll_settle`) that fails against the old code.
- **Bito's "Documentation contradicts Celery API" (filtered/muted, screenshot_utils.py:96)**: this turned out to be a real bug, not just docs — `current_task.request.timelimit` is `(time_limit, soft_time_limit)`, i.e. `(hard, soft)`, confirmed against `celery.app.amqp`/`celery.app.task.Context`/`celery.worker.strategy` source. My original unpacking had it backwards. Joe independently caught and fixed the same thing in 245cb8b before I got to it — my rebase picked that up, and I aligned the mock/tests to match Celery's real tuple order (not just my implementation) so a regression here would fail the test.
- **Bito's "test coverage gap for `log_context`" (webdriver.py)**: added `test_tiled_path_forwards_non_none_log_context`, asserting a non-`None` `log_context` passed to `WebDriverPlaywright.get_screenshot()` reaches `take_tiled_screenshot()` unchanged.
- **CodeAnt's type-annotation nits** on test fixtures/methods: leaving these as-is per your comment that they're not necessary.
- Pre-commit / ruff-format: clean against the pinned ruff 0.9.7 (verified directly, not just my sandbox's older local ruff, which briefly fought your formatting fix in the other direction — reverted before pushing).
Pushed in bcb5957ff9. CI failures I see now (`playwright-tests-experimental`) look unrelated — an embedded-dashboard pivot-table test, not touching this code path.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42118:
URL: https://github.com/apache/superset/pull/42118#issuecomment-5054221725
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #42118:
URL: https://github.com/apache/superset/pull/42118#issuecomment-5053233770
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/42118?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `13.58025%` with `70 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 56.01%. Comparing base ([`ef459c2`](https://app.codecov.io/gh/apache/superset/commit/ef459c21e8293f5a7669a05a9a1bd9fbbfb19d65?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`d64e368`](https://app.codecov.io/gh/apache/superset/commit/d64e368bf4caaeda272418adf40d71499fccdd78?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/42118?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [superset/utils/screenshot\_utils.py](https://app.codecov.io/gh/apache/superset/pull/42118?src=pr&el=tree&filepath=superset%2Futils%2Fscreenshot_utils.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvdXRpbHMvc2NyZWVuc2hvdF91dGlscy5weQ==) | 17.30% | [43 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/42118?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [superset/utils/webdriver.py](https://app.codecov.io/gh/apache/superset/pull/42118?src=pr&el=tree&filepath=superset%2Futils%2Fwebdriver.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvdXRpbHMvd2ViZHJpdmVyLnB5) | 10.00% | [18 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/42118?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [superset/utils/screenshots.py](https://app.codecov.io/gh/apache/superset/pull/42118?src=pr&el=tree&filepath=superset%2Futils%2Fscreenshots.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvdXRpbHMvc2NyZWVuc2hvdHMucHk=) | 0.00% | [9 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/42118?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (ef459c2) and HEAD (d64e368). Click for more details.
>
> <details><summary>HEAD has 15 uploads less than BASE</summary>
>
>| Flag | BASE (ef459c2) | HEAD (d64e368) |
>|------|------|------|
>|python|14|1|
>|presto|3|1|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #42118 +/- ##
==========================================
- Coverage 65.27% 56.01% -9.27%
==========================================
Files 2793 2793
Lines 157258 157307 +49
Branches 35953 35957 +4
==========================================
- Hits 102655 88115 -14540
- Misses 52625 68368 +15743
+ Partials 1978 824 -1154
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/42118/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/42118/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/42118/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/42118/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/42118/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `40.39% <13.58%> (-0.02%)` | :arrow_down: |
| [python](https://app.codecov.io/gh/apache/superset/pull/42118/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `40.39% <13.58%> (-18.77%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/42118/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/42118/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/42118?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #42118:
URL: https://github.com/apache/superset/pull/42118#issuecomment-5006213599
Thanks @eschutho, this looks like a solid fix.
Pre-commit's failing since `ruff-format` wants to reformat `webdriver_test.py`, so a `pre-commit run --all-files` (or using the pre-commit hook when committing locally) should clear that.
The rest of the failures (changes, frontend-build, playwright, etc.) look like the change-detector step hit a GitHub 503, so that's probably just a flaky rerun rather than anything in the diff. I just opened a PR for this separately so it retries.
Codeant's logic-error thread on the stale `remaining_budget` calc (computed before the scroll-settle sleep) is worth a look. The type-annotation nits from the same bot I wouldn't necessarily worry about, but there's a pile of open bot threads to assess/address/resolve, if you don't mind.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #42118:
URL: https://github.com/apache/superset/pull/42118#discussion_r3599301065
##########
tests/unit_tests/utils/test_screenshot_utils.py:
##########
@@ -397,3 +398,106 @@ def test_animation_wait_default_is_zero(self):
sig = inspect.signature(take_tiled_screenshot)
assert sig.parameters["animation_wait"].default == 0
+
+
+class TestTileWaitBudget:
+ @pytest.fixture
+ def mock_page(self):
Review Comment:
**Suggestion:** Add explicit parameter and return type annotations to this fixture method signature. [custom_rule]
**Severity Level:** Minor 🧹
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The new fixture method omits both parameter and return type annotations, which violates the Python type-hints rule for newly added code.
</details>
<details>
<summary><b>Rule source 📖 </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=7b38d85cc459435fb20d574556994ca5&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=7b38d85cc459435fb20d574556994ca5&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/utils/test_screenshot_utils.py
**Line:** 405:405
**Comment:**
*Custom Rule: Add explicit parameter and return type annotations to this fixture method signature.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42118&comment_hash=809943a70e337a20fbfc77808b8ffea53f6a9596232755f9a521be90ced6ef62&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42118&comment_hash=809943a70e337a20fbfc77808b8ffea53f6a9596232755f9a521be90ced6ef62&reaction=dislike'>👎</a>
##########
superset/utils/screenshot_utils.py:
##########
@@ -28,6 +29,35 @@
# Time to wait after scrolling for content to settle and load (in milliseconds)
SCROLL_SETTLE_TIMEOUT_MS = 1000
+# Total wall-clock budget, in seconds, for the entire tiled-screenshot
+# operation (element lookup plus all per-tile spinner/animation waits
+# combined). Each tile's wait is capped at whatever remains of this budget
+# so a slow dashboard degrades gracefully instead of running past the
+# Celery task time limit and getting SIGKILLed mid-capture.
+#
+# This is a conservative fixed constant rather than a value derived from
+# Celery's configured task time limit at runtime, because that limit isn't
+# reliably reachable from here: superset/tasks/scheduler.py sets it
+# per-report-schedule via `apply_async(time_limit=..., soft_time_limit=...)`
+# only when a schedule defines `working_timeout`
+# (ALERT_REPORTS_WORKING_TIME_OUT_KILL); there is no static config value
+# that always reflects the limit actually enforced, and this function can
+# also run outside of a Celery task entirely (e.g. synchronous thumbnail
+# generation).
+#
+# Production has observed a Celery hard task_time_limit of 1740s (29 min)
+# for report execution (2026-07-13 incident: a tiled screenshot was killed
+# mid-capture with SoftTimeLimitExceeded). This budget leaves a 300s margin
+# under that ceiling for the rest of the pipeline that runs after tiling
+# completes: combining tiles into one image, building the PDF, and
+# uploading/delivering the notification.
+TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS = 1440 # 1740s limit - 300s margin
Review Comment:
**Suggestion:** Add an explicit type annotation to this new module-level constant (for example, annotate it as an integer) to satisfy the type-hint requirement for relevant variables. [custom_rule]
**Severity Level:** Minor 🧹
<details>
<summary><b>Why it matters? ⭐ </b></summary>
This is a new module-level constant introduced without a type annotation, and the custom rule requires type hints for relevant variables that can be annotated. An `int` annotation would satisfy the rule.
</details>
<details>
<summary><b>Rule source 📖 </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=968703c5289f4b0a8e831cc1c71991bc&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=968703c5289f4b0a8e831cc1c71991bc&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/utils/screenshot_utils.py
**Line:** 54:54
**Comment:**
*Custom Rule: Add an explicit type annotation to this new module-level constant (for example, annotate it as an integer) to satisfy the type-hint requirement for relevant variables.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42118&comment_hash=da7aaf4f7698a094c3f4e87e27fd64b35811c2a91914d43f3602e3b8034acc10&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42118&comment_hash=da7aaf4f7698a094c3f4e87e27fd64b35811c2a91914d43f3602e3b8034acc10&reaction=dislike'>👎</a>
##########
tests/unit_tests/utils/test_screenshot_utils.py:
##########
@@ -397,3 +398,106 @@ def test_animation_wait_default_is_zero(self):
sig = inspect.signature(take_tiled_screenshot)
assert sig.parameters["animation_wait"].default == 0
+
+
+class TestTileWaitBudget:
+ @pytest.fixture
+ def mock_page(self):
+ """Create a mock Playwright page object for a 3-tile (5000px) dashboard."""
+ page = MagicMock()
+ element = MagicMock()
+ page.locator.return_value = element
+ page.evaluate.return_value = {
+ "height": 5000,
+ "top": 100,
+ "left": 50,
+ "width": 800,
+ }
+ page.screenshot.return_value = b"fake_screenshot_data"
+ return page
+
+ def test_per_tile_wait_shrinks_as_budget_depletes(self, mock_page, monkeypatch):
Review Comment:
**Suggestion:** Add type annotations for all parameters and an explicit return type on this new test method. [custom_rule]
**Severity Level:** Minor 🧹
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The added test method has unannotated parameters and no return type annotation, so it matches the type-hints violation rule.
</details>
<details>
<summary><b>Rule source 📖 </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f20f628fd0114eca86e2609f05bd71c8&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=f20f628fd0114eca86e2609f05bd71c8&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/utils/test_screenshot_utils.py
**Line:** 419:419
**Comment:**
*Custom Rule: Add type annotations for all parameters and an explicit return type on this new test method.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42118&comment_hash=a9f2206e01e67d3c1c64a79a1454d100716b66a8ea0dc3208814eeebc6f008a1&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42118&comment_hash=a9f2206e01e67d3c1c64a79a1454d100716b66a8ea0dc3208814eeebc6f008a1&reaction=dislike'>👎</a>
##########
tests/unit_tests/utils/test_screenshot_utils.py:
##########
@@ -397,3 +398,106 @@ def test_animation_wait_default_is_zero(self):
sig = inspect.signature(take_tiled_screenshot)
assert sig.parameters["animation_wait"].default == 0
+
+
+class TestTileWaitBudget:
+ @pytest.fixture
+ def mock_page(self):
+ """Create a mock Playwright page object for a 3-tile (5000px) dashboard."""
+ page = MagicMock()
+ element = MagicMock()
+ page.locator.return_value = element
+ page.evaluate.return_value = {
+ "height": 5000,
+ "top": 100,
+ "left": 50,
+ "width": 800,
+ }
+ page.screenshot.return_value = b"fake_screenshot_data"
+ return page
+
+ def test_per_tile_wait_shrinks_as_budget_depletes(self, mock_page, monkeypatch):
+ """Each tile's spinner-wait timeout is capped at the remaining budget."""
+ monkeypatch.setattr(
+ "superset.utils.screenshot_utils.TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS", # noqa: E501
+ 1000,
+ )
+ # monotonic() is called once for start_time, then once per tile to
+ # compute elapsed/remaining budget before that tile's spinner wait.
+ monotonic_values = iter([0, 0, 950, 990])
+ with patch(
+ "superset.utils.screenshot_utils.time.monotonic",
+ side_effect=lambda: next(monotonic_values),
+ ):
+ with patch("superset.utils.screenshot_utils.combine_screenshot_tiles"):
+ result = take_tiled_screenshot(
+ mock_page, "dashboard", tile_height=2000, load_wait=100
+ )
+
+ assert result is not None
+ timeouts = [
+ call[1]["timeout"] for call in mock_page.wait_for_function.call_args_list
+ ]
+ # remaining budget: 1000, 50, 10 seconds -> capped timeouts shrink
+ assert timeouts == [100 * 1000, 50 * 1000, 10 * 1000]
+ assert timeouts == sorted(timeouts, reverse=True)
+
+ def test_budget_exhausted_raises_and_stops_capturing(self, mock_page, monkeypatch):
+ """Exhausting the budget aborts cleanly instead of capturing unchecked."""
+ monkeypatch.setattr(
+ "superset.utils.screenshot_utils.TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS", # noqa: E501
+ 1000,
+ )
+ # start_time=0, tile 0 check: elapsed=0 (proceeds and captures),
+ # tile 1 check: elapsed=1000 -> remaining=0 -> raise before capturing.
+ monotonic_values = iter([0, 0, 1000])
+ with patch(
+ "superset.utils.screenshot_utils.time.monotonic",
+ side_effect=lambda: next(monotonic_values),
+ ):
+ with patch(
+ "superset.utils.screenshot_utils.combine_screenshot_tiles"
+ ) as mock_combine:
+ with patch("superset.utils.screenshot_utils.logger") as mock_logger:
+ with pytest.raises(TiledScreenshotBudgetExceededError):
+ take_tiled_screenshot(
+ mock_page, "dashboard", tile_height=2000, load_wait=100
+ )
+
+ # Only the first tile was captured before the budget ran out.
+ assert mock_page.screenshot.call_count == 1
+ # Tiles were never combined -- the function raised before that point.
+ mock_combine.assert_not_called()
+
+ mock_logger.error.assert_called_once()
+ error_args = mock_logger.error.call_args[0]
+ assert "budget exhausted" in error_args[0]
+ # tiles captured, tiles total, elapsed seconds, budget seconds
+ assert error_args[1] == 1
+ assert error_args[2] == 3
+ assert error_args[3] == 1000
+ assert error_args[4] == 1000
+
+ def test_fast_dashboard_matches_default_behavior(self, mock_page):
Review Comment:
**Suggestion:** Add parameter and return type annotations to this test method declaration. [custom_rule]
**Severity Level:** Minor 🧹
<details>
<summary><b>Why it matters? ⭐ </b></summary>
This new test method omits type hints for its argument and return value, so the suggestion correctly identifies a type-hints violation.
</details>
<details>
<summary><b>Rule source 📖 </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6de36cb75c8447d7b86ccff9fe4602ac&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=6de36cb75c8447d7b86ccff9fe4602ac&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/utils/test_screenshot_utils.py
**Line:** 481:481
**Comment:**
*Custom Rule: Add parameter and return type annotations to this test method declaration.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42118&comment_hash=57588b945dc2632369429cf0ff516110c40c922e4d36f1ae8add364cf23ca0fa&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42118&comment_hash=57588b945dc2632369429cf0ff516110c40c922e4d36f1ae8add364cf23ca0fa&reaction=dislike'>👎</a>
##########
tests/unit_tests/utils/test_screenshot_utils.py:
##########
@@ -397,3 +398,106 @@ def test_animation_wait_default_is_zero(self):
sig = inspect.signature(take_tiled_screenshot)
assert sig.parameters["animation_wait"].default == 0
+
+
+class TestTileWaitBudget:
+ @pytest.fixture
+ def mock_page(self):
+ """Create a mock Playwright page object for a 3-tile (5000px) dashboard."""
+ page = MagicMock()
+ element = MagicMock()
+ page.locator.return_value = element
+ page.evaluate.return_value = {
+ "height": 5000,
+ "top": 100,
+ "left": 50,
+ "width": 800,
+ }
+ page.screenshot.return_value = b"fake_screenshot_data"
+ return page
+
+ def test_per_tile_wait_shrinks_as_budget_depletes(self, mock_page, monkeypatch):
+ """Each tile's spinner-wait timeout is capped at the remaining budget."""
+ monkeypatch.setattr(
+ "superset.utils.screenshot_utils.TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS", # noqa: E501
+ 1000,
+ )
+ # monotonic() is called once for start_time, then once per tile to
+ # compute elapsed/remaining budget before that tile's spinner wait.
+ monotonic_values = iter([0, 0, 950, 990])
+ with patch(
+ "superset.utils.screenshot_utils.time.monotonic",
+ side_effect=lambda: next(monotonic_values),
+ ):
+ with patch("superset.utils.screenshot_utils.combine_screenshot_tiles"):
+ result = take_tiled_screenshot(
+ mock_page, "dashboard", tile_height=2000, load_wait=100
+ )
+
+ assert result is not None
+ timeouts = [
+ call[1]["timeout"] for call in mock_page.wait_for_function.call_args_list
+ ]
+ # remaining budget: 1000, 50, 10 seconds -> capped timeouts shrink
+ assert timeouts == [100 * 1000, 50 * 1000, 10 * 1000]
+ assert timeouts == sorted(timeouts, reverse=True)
+
+ def test_budget_exhausted_raises_and_stops_capturing(self, mock_page, monkeypatch):
Review Comment:
**Suggestion:** Add type hints to this method signature, including fixture argument types and a return type. [custom_rule]
**Severity Level:** Minor 🧹
<details>
<summary><b>Why it matters? ⭐ </b></summary>
This newly added test method is missing type annotations on its parameters and return type, which is a real violation of the type-hints rule.
</details>
<details>
<summary><b>Rule source 📖 </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8dae0036fa864eefb48dfb6676e4a31a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=8dae0036fa864eefb48dfb6676e4a31a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/utils/test_screenshot_utils.py
**Line:** 445:445
**Comment:**
*Custom Rule: Add type hints to this method signature, including fixture argument types and a return type.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42118&comment_hash=3de3388727e50825725e25aae3d21fbf3211ccdd9c76403f22557ff14ef7c776&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42118&comment_hash=3de3388727e50825725e25aae3d21fbf3211ccdd9c76403f22557ff14ef7c776&reaction=dislike'>👎</a>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #42118:
URL: https://github.com/apache/superset/pull/42118#issuecomment-5064300466
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | bcb5957ff9efde1a036c415f0d2c3b761ba1c9e9 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a629a1870ef9e000876d074 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-42118--superset-docs-preview.netlify.app](https://deploy-preview-42118--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#q1YqKMrPLShRslJS0lFKKkrMS84AstMyK3RLMnNSdcsTM0t0k0pT0lNLgPIpqQU5-ZWeKUAVZolmRpaJhhbmBqlplqkGBgYW5mYpBuYmSrUA) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-42118--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill [superset]
Posted by "rebenitez1802 (via GitHub)" <gi...@apache.org>.
rebenitez1802 commented on code in PR #42118:
URL: https://github.com/apache/superset/pull/42118#discussion_r3693643801
##########
superset/utils/screenshot_utils.py:
##########
@@ -286,6 +315,15 @@ def take_tiled_screenshot(
# match `except PlaywrightTimeout` and incorrectly propagate instead of
# degrading to `None` like every other unexpected error in this function.
readiness_timeout = False
+ # Cap the whole tiled operation against the running Celery task's own
+ # time limit, using the same runtime derivation as the non-tiled
+ # readiness wait (#42253/#42427). Unlike that path, a None budget does
+ # not mean "keep the configured timeout": per-tile waits accumulate, so
+ # the operation falls back to a fixed total ceiling instead.
+ wait_budget_seconds = resolve_screenshot_task_budget_seconds(log_context)
+ if wait_budget_seconds is None:
+ wait_budget_seconds = float(TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS)
+ start_time = time.monotonic()
Review Comment:
**🟡 Non-blocking (owner's call) — tiled budget clock resets instead of accounting for pre-capture time.**
`wait_budget_seconds` is derived from the Celery task limit, but elapsed is measured from this local `start_time`, so `page.goto` (bounded 60s), the 3s `SELENIUM_HEADSTART`, the 30s `element.wait_for`, and dimension probing all run *before* the clock starts — the tiled path effectively gets a fresh full budget. The non-tiled `_wait_for_charts_ready` avoids this by threading `screenshot_started_at` from the top of `get_screenshot` and subtracting already-elapsed time; the tiled call site doesn't pass it.
Not a correctness bug: in the common case the 20%/300s margin (and the soft→hard gap) absorbs the difference, and a soft-limit overrun is still caught cleanly as `SoftTimeLimitExceeded` rather than the SIGKILL this PR targets. It's just a looser guarantee than the non-tiled path — worth a conscious decision.
If you want both paths on one clock, it's 3 coordinated one-liners (⚠️ applying the suggestion below *alone* will `NameError` until the other two land):
1. add `screenshot_started_at: float | None = None` to the `take_tiled_screenshot` signature;
2. the change below;
3. pass `screenshot_started_at=screenshot_started_at` from the `take_tiled_screenshot(...)` call in `webdriver.py`.
```suggestion
start_time = (
screenshot_started_at if screenshot_started_at is not None else time.monotonic()
)
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #42118:
URL: https://github.com/apache/superset/pull/42118#discussion_r3692396771
##########
superset/utils/screenshot_utils.py:
##########
@@ -377,12 +471,36 @@ def take_tiled_screenshot(
load_wait,
context_suffix,
)
+ readiness_wait_elapsed = time.monotonic() - tile_wait_start
# Wait for chart animations (e.g. ECharts) to finish after spinner clears.
# The global animation wait before tiling only covers the first tile;
- # subsequent tiles need their own wait after data loads.
+ # subsequent tiles need their own wait after data loads. Capped at
+ # whatever remains of the budget; unlike the readiness wait above this
+ # is cosmetic settling, not a readiness check, so we simply skip it
+ # (rather than raise) once the budget runs out.
+ animation_wait_elapsed = 0.0
if animation_wait > 0:
- page.wait_for_timeout(animation_wait * 1000)
+ elapsed = time.monotonic() - start_time
+ remaining_budget = wait_budget_seconds - elapsed
+ tile_animation_wait = max(0, min(animation_wait, remaining_budget))
+ if tile_animation_wait > 0:
+ animation_wait_start = time.monotonic()
+ page.wait_for_timeout(tile_animation_wait * 1000)
+ animation_wait_elapsed = time.monotonic() - animation_wait_start
Review Comment:
**Suggestion:** After the capped animation wait completes, the code proceeds directly to `page.screenshot()` without checking the remaining budget. If the animation wait consumes the final available time, the tile is still captured after the hard budget has expired, and if this is the last tile the function returns successfully despite exceeding the task limit. Recheck the budget before capture and raise `TiledScreenshotBudgetExceededError` when it is exhausted. [incorrect control flow]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Tiled report capture can exceed its safe task budget.
- ⚠️ Celery cleanup time can be consumed by screenshot capture.
- ⚠️ Last tiles may succeed after budget exhaustion.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=452182239d354cd7b752b155df6d37aa&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=452182239d354cd7b752b155df6d37aa&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/utils/screenshot_utils.py
**Line:** 482:490
**Comment:**
*Incorrect Control Flow: After the capped animation wait completes, the code proceeds directly to `page.screenshot()` without checking the remaining budget. If the animation wait consumes the final available time, the tile is still captured after the hard budget has expired, and if this is the last tile the function returns successfully despite exceeding the task limit. Recheck the budget before capture and raise `TiledScreenshotBudgetExceededError` when it is exhausted.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42118&comment_hash=425fcb1612db3969bc016b8e094f165af558b6154f2491d4fbc37f3bf13e6510&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42118&comment_hash=425fcb1612db3969bc016b8e094f165af558b6154f2491d4fbc37f3bf13e6510&reaction=dislike'>👎</a>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42118:
URL: https://github.com/apache/superset/pull/42118#issuecomment-5148718640
**Bito Automatic Review Skipped – PR Already Merged**<br/><br/>Bito scheduled an automatic review for this pull request, but the review was skipped because this PR was merged before the review could be run.<br/>No action is needed if you didn't intend to review it. To get a review, you can type `/review` in a comment and save it
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #42118:
URL: https://github.com/apache/superset/pull/42118#discussion_r3634941295
##########
superset/utils/screenshot_utils.py:
##########
@@ -217,40 +334,80 @@ def take_tiled_screenshot(
dashboard_top = element_info["top"]
logger.info(
- "Dashboard: %sx%spx at (%s, %s)",
+ "Dashboard: %sx%spx at (%s, %s)%s",
dashboard_width,
dashboard_height,
dashboard_left,
dashboard_top,
+ context_suffix,
)
# Calculate number of tiles needed
num_tiles = max(1, (dashboard_height + tile_height - 1) // tile_height)
- logger.info("Taking %s screenshot tiles", num_tiles)
+ logger.info("Taking %s screenshot tiles%s", num_tiles, context_suffix)
- screenshot_tiles = []
+ screenshot_tiles: list[bytes] = []
for i in range(num_tiles):
+ # Check the time budget before starting this tile's readiness wait.
+ # If it's already exhausted, we can no longer verify this (or any
+ # later) tile is actually ready to capture -- fail loudly instead
+ # of silently snapshotting a spinner or blank chart, or running
+ # past the Celery task time limit and getting SIGKILLed.
+ tile_start = time.monotonic()
+ elapsed = tile_start - start_time
+ remaining_budget = wait_budget_seconds - elapsed
+ if remaining_budget <= 0:
+ # A customer-side chart-loading issue (a slow/hung dashboard),
+ # not a Superset system fault, so this is a WARNING rather
+ # than an ERROR -- consistent with #38130/#38441, which
+ # deliberately downgraded screenshot timeout logs the same way.
+ logger.warning(
+ "Tiled screenshot time budget exhausted on tile %s/%s: "
+ "%s/%s tiles captured so far, %.1fs elapsed of a %.1fs "
+ "budget. Aborting instead of capturing remaining tiles "
+ "unchecked.%s",
+ i + 1,
+ num_tiles,
+ len(screenshot_tiles),
+ num_tiles,
+ elapsed,
+ wait_budget_seconds,
+ context_suffix,
+ )
+ raise TiledScreenshotBudgetExceededError(
+ f"Tiled screenshot budget of "
+ f"{wait_budget_seconds:.1f}s exhausted "
+ f"after {len(screenshot_tiles)}/{num_tiles} tiles"
+ )
+
# Calculate scroll position to show this tile's content
scroll_y = dashboard_top + (i * tile_height)
page.evaluate(f"window.scrollTo(0, {scroll_y})")
logger.debug(
- "Scrolled window to %s for tile %s/%s", scroll_y, i + 1, num_tiles
+ "Scrolled window to %s for tile %s/%s%s",
+ scroll_y,
+ i + 1,
+ num_tiles,
+ context_suffix,
)
# Wait for scroll to settle and content to load
page.wait_for_timeout(SCROLL_SETTLE_TIMEOUT_MS)
# Wait for every chart holder visible in the current viewport to reach
- # a terminal state (rendered chart or error/empty state). Only check
+ # a terminal state (rendered chart or error/empty state), capped at
+ # whatever remains of the total time budget so a slow dashboard
+ # degrades gracefully instead of exceeding it. Only check
# viewport-visible chart holders to avoid blocking on virtualization
# placeholders rendered for off-screen charts. A holder that hasn't
# mounted anything yet does not satisfy this check -- unlike checking
# for the absence of `.loading`, which passes vacuously in that case.
+ tile_load_wait = min(load_wait, remaining_budget)
Review Comment:
**Suggestion:** The remaining budget is computed before the mandatory scroll-settle delay, but then reused for `wait_for_function` without recalculating. When the budget is nearly exhausted, this can overrun the intended total budget by at least the settle delay (and potentially trigger Celery time-limit kills again). Recompute `remaining_budget` after `page.wait_for_timeout(...)` (or include settle wait in the cap) before deriving the per-tile readiness timeout. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Tiled screenshot budget enforcement can overshoot configured budget.
- ⚠️ Long-running report captures may still approach Celery limits.
- ⚠️ Logging claims strict capping but implementation exceeds budget.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In `superset/utils/screenshot_utils.py`, note the tiled capture loop in
`take_tiled_screenshot()` starting around line 351, where `tile_start`, `elapsed`, and
`remaining_budget` are computed (lines 356-359) and checked against zero (line 360).
2. Still in the same function, observe that after the budget check, the code scrolls and
then waits for scroll settle via `page.wait_for_timeout(SCROLL_SETTLE_TIMEOUT_MS)` (lines
385-396), and only afterwards calls `page.wait_for_function(...)` using `tile_load_wait =
min(load_wait, remaining_budget)` (line 405), where `remaining_budget` is the value
computed before the settle wait.
3. Write a unit test or small harness that calls `take_tiled_screenshot()` (function
defined starting around line 272 in this file) with:
- `_resolve_wait_budget_seconds()` (lines 80-133) patched to return a small budget, for
example 2.0 seconds.
- `load_wait` set to a larger value, for example 60.
- A fake or real Playwright `Page` whose `wait_for_timeout` implementation actually
sleeps for `SCROLL_SETTLE_TIMEOUT_MS` (1000 milliseconds), and whose
`wait_for_function` can also sleep for the requested timeout.
4. In that test, drive enough elapsed time before entering a given tile iteration (for
example by patching `time.monotonic()` or by performing real sleeps) so that, at the
budget check on line 356, `remaining_budget` is a small positive value (for example 0.1
seconds). Then let the code run:
- The loop passes the `remaining_budget > 0` check and logs the warning context.
- It calls `page.wait_for_timeout(SCROLL_SETTLE_TIMEOUT_MS)` (line 396), consuming 1.0
additional seconds while still holding the stale `remaining_budget` value.
- It then computes `tile_load_wait = min(load_wait, remaining_budget)` (line 405) using
that stale value and passes it to `page.wait_for_function`, so the total elapsed time
since `start_time` exceeds the intended budget `wait_budget_seconds` by at least the
scroll-settle duration, demonstrating that the total budget is not actually capped by
`remaining_budget` as documented.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e0092aafcf2b41a89f990519fc5c554c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=e0092aafcf2b41a89f990519fc5c554c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/utils/screenshot_utils.py
**Line:** 356:405
**Comment:**
*Logic Error: The remaining budget is computed before the mandatory scroll-settle delay, but then reused for `wait_for_function` without recalculating. When the budget is nearly exhausted, this can overrun the intended total budget by at least the settle delay (and potentially trigger Celery time-limit kills again). Recompute `remaining_budget` after `page.wait_for_timeout(...)` (or include settle wait in the cap) before deriving the per-tile readiness timeout.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42118&comment_hash=199ee2e887c1db464fe4efda295a6f824f32707ac82e1ea8f4002bb35fb58255&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42118&comment_hash=199ee2e887c1db464fe4efda295a6f824f32707ac82e1ea8f4002bb35fb58255&reaction=dislike'>👎</a>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42118:
URL: https://github.com/apache/superset/pull/42118#issuecomment-5061192817
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org