alexandrusoare commented on code in PR #44082:
URL: https://github.com/apache/superset/pull/44082#discussion_r3977848385
##########
superset/dashboards/api.py:
##########
@@ -1825,25 +1849,115 @@ def export_xlsx(self, pk: int) -> WerkzeugResponse:
)
job_id = str(uuid.uuid4())
+ if queued:
+ return self._export_xlsx_queued(
+ dashboard, active_data_mask, mode, job_id, lock_params
+ )
+
+ # Plan after locking because query-context resolution can be expensive.
+ # Release here unless the inline exporter takes over cleanup.
+ lock_delegated = False
+ try:
+ plan: InlineExportPlan = plan_inline_export(dashboard)
+ if not plan.fits_row_budget:
+ return self.response_400(
+ message=(
+ "This dashboard requests too many rows to export in a "
+ "single request. Configure EXCEL_EXPORT_S3_BUCKET to "
+ "export it in the background, or lower the row limits
of "
+ "its charts."
+ )
+ )
+ lock_delegated = True
+ return self._export_xlsx_inline(
+ dashboard,
+ active_data_mask,
+ job_id,
+ lock_params,
+ plan.query_contexts,
+ )
+ finally:
+ if not lock_delegated:
+ try:
+ ReleaseDistributedLock(EXPORT_LOCK_NAMESPACE,
lock_params).run()
+ except Exception: # pylint: disable=broad-except
+ # The TTL is the fallback if release fails.
+ logger.exception(
+ "Failed to release in-flight export lock for dashboard
%s",
+ dashboard.id,
+ )
+
+ def _export_xlsx_queued( # pylint: disable=too-many-arguments
+ self,
+ dashboard: Dashboard,
+ active_data_mask: dict[str, Any],
+ mode: str,
+ job_id: str,
+ lock_params: dict[str, int],
+ ) -> WerkzeugResponse:
+ """Queue an export for upload and email delivery."""
try:
export_dashboard_excel.apply_async(
kwargs={
"dashboard_id": dashboard.id,
"user_id": g.user.id,
- "active_data_mask": payload.get("active_data_mask", {}),
+ "active_data_mask": active_data_mask,
"job_id": job_id,
- "mode": payload.get("mode", "data"),
+ "mode": mode,
},
task_id=job_id,
)
except Exception:
- # If enqueuing fails (e.g. broker down) the task will never run to
- # release the lock, so free it now rather than block exports until
- # the TTL expires.
+ # No task will release the lock if enqueueing fails.
ReleaseDistributedLock(EXPORT_LOCK_NAMESPACE, lock_params).run()
raise
return self.response(202, job_id=job_id)
+ @staticmethod
+ def _export_xlsx_inline( # pylint: disable=too-many-arguments
+ dashboard: Dashboard,
+ active_data_mask: dict[str, Any],
+ job_id: str,
+ lock_params: dict[str, int],
+ query_contexts: ResolvedQueryContexts,
+ ) -> WerkzeugResponse:
+ """Build a planned data export and return it in the response."""
+ tmp_path: str | None = None
+ try:
+ file_descriptor, tmp_path = tempfile.mkstemp(
+ suffix=".xlsx", prefix=f"dash-export-{job_id}-"
+ )
+ os.close(file_descriptor)
+
+ build_workbook(
+ tmp_path,
+ dashboard,
+ active_data_mask,
+ job_id,
+ EXPORT_MODE_DATA,
+ g.user,
+ query_contexts=query_contexts,
+ )
+ with open(tmp_path, "rb") as workbook:
Review Comment:
NIT: This reads the entire workbook into memory before responding; at 100k
rows that could be tens of MB per worker. Would flask.send_file(tmp_path, ...)
with deferred cleanup work here instead?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]