graceguo-supercat commented on a change in pull request #17749:
URL: https://github.com/apache/superset/pull/17749#discussion_r780454780
##########
File path: superset/reports/commands/create.py
##########
@@ -113,3 +114,20 @@ def validate(self) -> None:
exception = ReportScheduleInvalidError()
exception.add_list(exceptions)
raise exception
+
+ def _validate_report_extra(self, exceptions: List[ValidationError]) ->
None:
+ extra = self._properties.get("extra")
+ dashboard = self._properties.get("dashboard")
+
+ if extra is None or dashboard is None:
+ return
+
+ dashboard_tab_ids = extra.get("dashboard_tab_ids")
+ if dashboard_tab_ids is None:
+ return
+ position_data = json.loads(dashboard.position_json)
+ invalid_tab_ids = [
+ tab_id for tab_id in dashboard_tab_ids if tab_id not in
position_data
+ ]
+ if invalid_tab_ids:
+ exceptions.append(ValidationError("Invalid tab IDs selected",
"extra"))
Review comment:
i think let owners know error details is important. i added extra filed
in error message.
##########
File path: superset/reports/commands/execute.py
##########
@@ -187,34 +187,48 @@ def _get_user(self) -> User:
raise ReportScheduleSelleniumUserNotFoundError()
return user
- def _get_screenshot(self) -> bytes:
+ def _get_screenshots(self) -> List[bytes]:
"""
- Get a chart or dashboard screenshot
-
+ Get chart or dashboard screenshots
:raises: ReportScheduleScreenshotFailedError
"""
- screenshot: Optional[BaseScreenshot] = None
+ image_data = []
+ screenshots: List[BaseScreenshot] = []
if self._report_schedule.chart:
url = self._get_url()
logger.info("Screenshotting chart at %s", url)
- screenshot = ChartScreenshot(
- url,
- self._report_schedule.chart.digest,
- window_size=app.config["WEBDRIVER_WINDOW"]["slice"],
- thumb_size=app.config["WEBDRIVER_WINDOW"]["slice"],
- )
+ screenshots = [
+ ChartScreenshot(
+ url,
+ self._report_schedule.chart.digest,
+ window_size=app.config["WEBDRIVER_WINDOW"]["slice"],
+ thumb_size=app.config["WEBDRIVER_WINDOW"]["slice"],
+ )
+ ]
else:
- url = self._get_url()
- logger.info("Screenshotting dashboard at %s", url)
- screenshot = DashboardScreenshot(
- url,
- self._report_schedule.dashboard.digest,
- window_size=app.config["WEBDRIVER_WINDOW"]["dashboard"],
- thumb_size=app.config["WEBDRIVER_WINDOW"]["dashboard"],
+ tabs: Optional[List[str]] =
json.loads(self._report_schedule.extra).get(
+ "dashboard_tab_ids", None
)
+ dashboard_base_url = self._get_url()
+ if tabs is None:
+ urls = [dashboard_base_url]
+ else:
+ urls = [f"{dashboard_base_url}#{tab_id}" for tab_id in tabs]
+ screenshots = [
+ DashboardScreenshot(
+ url,
+ self._report_schedule.dashboard.digest,
+ window_size=app.config["WEBDRIVER_WINDOW"]["dashboard"],
+ thumb_size=app.config["WEBDRIVER_WINDOW"]["dashboard"],
+ )
+ for url in urls
+ ]
user = self._get_user()
try:
- image_data = screenshot.get_screenshot(user=user)
+ for screenshot in screenshots:
+ image = screenshot.get_screenshot(user=user)
+ if image is not None:
+ image_data.append(image)
except SoftTimeLimitExceeded as ex:
Review comment:
can we try add time limit on each of screenshot?
##########
File path: superset/reports/commands/create.py
##########
@@ -113,3 +114,20 @@ def validate(self) -> None:
exception = ReportScheduleInvalidError()
exception.add_list(exceptions)
raise exception
+
+ def _validate_report_extra(self, exceptions: List[ValidationError]) ->
None:
+ extra = self._properties.get("extra")
+ dashboard = self._properties.get("dashboard")
+
+ if extra is None or dashboard is None:
+ return
+
+ dashboard_tab_ids = extra.get("dashboard_tab_ids")
+ if dashboard_tab_ids is None:
+ return
+ position_data = json.loads(dashboard.position_json)
+ invalid_tab_ids = [
+ tab_id for tab_id in dashboard_tab_ids if tab_id not in
position_data
+ ]
+ if invalid_tab_ids:
+ exceptions.append(ValidationError("Invalid tab IDs selected",
"extra"))
Review comment:
i think let owners know which tab_id is invalid is important. So i
prefer to keep `invalid_tab_ids` and add it to error message.
--
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-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
