gabotorresruiz commented on code in PR #43805:
URL: https://github.com/apache/superset/pull/43805#discussion_r3960627288


##########
docs/docs/using-superset/exporting-dashboard-data.mdx:
##########
@@ -72,33 +91,48 @@ will not register.
 
 ## Configuration keys
 
-| Key                             | Default                | Description       
                                                                                
                                                                                
|
-| ------------------------------- | ---------------------- | 
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 |
-| `EXCEL_EXPORT_S3_BUCKET`        | `None`                 | Destination 
bucket. Required; `501` if unset.                                               
                                                                                
      |
-| `EXCEL_EXPORT_S3_KEY_PREFIX`    | `"dashboard-exports/"` | Key prefix: 
`{prefix}{dashboard_id}/{job_id}.xlsx`.                                         
                                                                                
      |
-| `EXCEL_EXPORT_LINK_TTL_SECONDS` | `86400`                | Lifetime of the 
pre-signed download URL (24h).                                                  
                                                                                
  |
-| `EXCEL_EXPORT_S3_CLIENT_KWARGS` | `{}`                   | Extra kwargs for 
`boto3.client("s3", ...)` — e.g. `region_name`, or `endpoint_url` for 
MinIO/LocalStack.                                                               
           |
-| `EXCEL_EXPORT_TABLE_VIZ_TYPES`  | `None`                 | Viz types kept 
tabular in **Export Images to Excel** mode; every other type is embedded as an 
image. `None` uses the built-in default (`table`, `pivot_table`, 
`pivot_table_v2`). |
-| `EXCEL_EXPORT_QUERY_CONTEXT_BUILDER` | `None`            | Optional 
`Callable[[form_data_dict], dict \| None]` to build a query context for a chart 
missing a saved one, tried before the built-in form-data rebuild. Point it at a 
service that runs the chart's real frontend `buildQuery` to faithfully export 
viz types the built-in rebuild can't handle. Must return `None` when it can't 
build faithfully, so the export falls back. |
-
-Credentials and region resolve through the standard boto3 chain (environment
-variables, shared config, or instance role) unless overridden via
-`EXCEL_EXPORT_S3_CLIENT_KWARGS`. The worker needs `s3:PutObject` on the bucket.
+| Key                                    | Default                | 
Description                                                                     
                                                                                
                  |
+| -------------------------------------- | ---------------------- | 
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 |
+| `EXPORT_STORAGE["bucket"]`             | unset                  | 
Destination bucket. Required; `501` if unset.                                   
                                                                                
                  |
+| `EXPORT_STORAGE["backend"]`            | unset                  | Storage 
backend instance: `S3ExportStorage()` (`superset.utils.s3`), 
`GCSExportStorage()` (`superset.utils.gcs`), or a custom 
`superset.utils.export_storage.ExportStorage` implementation. Required; `501` 
if unset. |
+| `EXPORT_STORAGE["key_prefix"]`         | `"dashboard-exports/"` | Object 
key/blob prefix: `{prefix}{dashboard_id}/{job_id}.xlsx`. A callable (`() -> 
str`) is invoked per export, for prefixes only known in request/task context 
(e.g. per-tenant scoping of a shared bucket). |

Review Comment:
   Fixed in b80a6e02dd: the key_prefix docs row now states the callable runs 
inside the Celery worker with app context only (reading flask.request fails 
every export) and that per tenant prefixes should come from worker ambient app 
config.



##########
superset-frontend/src/dashboard/components/menu/DownloadMenuItems/index.tsx:
##########
@@ -167,6 +211,75 @@ export const useDownloadMenuItems = (
     }
   };
 
+  const pollExportStatus = (jobId: string, pollState: ExportPollState) => {
+    if (unmountedRef.current) {
+      return;
+    }
+    SupersetClient.get({
+      endpoint: `/api/v1/dashboard/export_xlsx/status/${jobId}/`,
+    })
+      .then(({ json }) => {
+        // A response in flight when the component unmounts must not navigate
+        // (redirect) or toast on whatever page the user moved to.
+        if (unmountedRef.current) {
+          return;
+        }
+        const {
+          status,
+          download_url: downloadUrl,
+          message,
+        } = json as ExportStatusResponse;
+        if (status === 'ready') {
+          if (downloadUrl) {
+            redirect(downloadUrl);

Review Comment:
   Fixed in b80a6e02dd: the auto download now fetches the file and saves it via 
an anchor, mirroring the Export as Example pattern in the same file, so a dead 
link surfaces a retryable error toast instead of replacing the dashboard with a 
JSON response. Covered by a new jest test for the dead link path, and the ready 
and running tests now assert the fetch based download.



##########
docs/docs/using-superset/exporting-dashboard-data.mdx:
##########
@@ -72,33 +91,48 @@ will not register.
 
 ## Configuration keys
 
-| Key                             | Default                | Description       
                                                                                
                                                                                
|
-| ------------------------------- | ---------------------- | 
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 |
-| `EXCEL_EXPORT_S3_BUCKET`        | `None`                 | Destination 
bucket. Required; `501` if unset.                                               
                                                                                
      |
-| `EXCEL_EXPORT_S3_KEY_PREFIX`    | `"dashboard-exports/"` | Key prefix: 
`{prefix}{dashboard_id}/{job_id}.xlsx`.                                         
                                                                                
      |
-| `EXCEL_EXPORT_LINK_TTL_SECONDS` | `86400`                | Lifetime of the 
pre-signed download URL (24h).                                                  
                                                                                
  |
-| `EXCEL_EXPORT_S3_CLIENT_KWARGS` | `{}`                   | Extra kwargs for 
`boto3.client("s3", ...)` — e.g. `region_name`, or `endpoint_url` for 
MinIO/LocalStack.                                                               
           |
-| `EXCEL_EXPORT_TABLE_VIZ_TYPES`  | `None`                 | Viz types kept 
tabular in **Export Images to Excel** mode; every other type is embedded as an 
image. `None` uses the built-in default (`table`, `pivot_table`, 
`pivot_table_v2`). |
-| `EXCEL_EXPORT_QUERY_CONTEXT_BUILDER` | `None`            | Optional 
`Callable[[form_data_dict], dict \| None]` to build a query context for a chart 
missing a saved one, tried before the built-in form-data rebuild. Point it at a 
service that runs the chart's real frontend `buildQuery` to faithfully export 
viz types the built-in rebuild can't handle. Must return `None` when it can't 
build faithfully, so the export falls back. |
-
-Credentials and region resolve through the standard boto3 chain (environment
-variables, shared config, or instance role) unless overridden via
-`EXCEL_EXPORT_S3_CLIENT_KWARGS`. The worker needs `s3:PutObject` on the bucket.
+| Key                                    | Default                | 
Description                                                                     
                                                                                
                  |
+| -------------------------------------- | ---------------------- | 
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 |
+| `EXPORT_STORAGE["bucket"]`             | unset                  | 
Destination bucket. Required; `501` if unset.                                   
                                                                                
                  |
+| `EXPORT_STORAGE["backend"]`            | unset                  | Storage 
backend instance: `S3ExportStorage()` (`superset.utils.s3`), 
`GCSExportStorage()` (`superset.utils.gcs`), or a custom 
`superset.utils.export_storage.ExportStorage` implementation. Required; `501` 
if unset. |
+| `EXPORT_STORAGE["key_prefix"]`         | `"dashboard-exports/"` | Object 
key/blob prefix: `{prefix}{dashboard_id}/{job_id}.xlsx`. A callable (`() -> 
str`) is invoked per export, for prefixes only known in request/task context 
(e.g. per-tenant scoping of a shared bucket). |

Review Comment:
   Good catch, the docs table still described the old contract. Updated in 
b80a6e02dd: the callable runs in the worker with no request context, so per 
tenant prefixes should come from app config.



-- 
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]

Reply via email to