You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "seunggabi (via GitHub)" <gi...@apache.org> on 2026/09/22 02:18:50 UTC

[I] [SIP] Proposal for an optional download reason on data exports [superset]

seunggabi opened a new issue, #44512:
URL: https://github.com/apache/superset/issues/44512

   ## [SIP] Proposal for an optional download reason on data exports
   
   ### Motivation
   
   Superset's Action Log (`logs.json`) already records *who* exported *which* chart or query result and *when*. It does not record *why*.
   
   For some deployments the "why" is a compliance requirement, not a nice-to-have. Korea's PIPA safeguards standard (개인정보의 안전성 확보조치 기준, Art. 8) requires that when access logs show personal data was downloaded, the controller checks and records the reason for that download. GDPR and HIPAA style audits ask the same "purpose of processing" question. Today that answer lives outside Superset: a spreadsheet next to the log, or a security team chasing people after the monthly log review ("you exported 30k rows of the customer table on the 12th, what was that for?").
   
   Examples of what the log should be able to say:
   
   - `FIN-1123 churn deck for finance` on a CSV export of a customer-segment chart
   - `checking a suspected dup in orders, will delete after` on a SQL Lab result download
   - `Scheduled report: Weekly KPI (id=42)` on an xlsx pulled by Alerts & Reports, so report-driven downloads are separable from human ones
   
   A side effect operators have asked for: the prompt itself discourages casual "download everything".
   
   ### Proposed Change
   
   Implemented in #44499, opt-in behind a feature flag `REQUIRE_DOWNLOAD_REASON` (default `False`).
   
   When the flag is on:
   
   - **Frontend**: before a CSV/XLSX export from Explore, a dashboard chart menu, the dashboard-wide Excel export, or SQL Lab (regular and streaming), a small dialog asks "Why are you downloading this data?". Cancel aborts the export. The trimmed reason (max 255 chars) is sent as the `download_reason` query parameter. JSON / image / PDF exports are not affected.
   - **Backend**: the CSV/XLSX export endpoints reject requests without a non-blank `download_reason` with a SIP-40 `400` error. Nothing new is persisted: the event logger already merges query-string and form fields into the `logs.json` payload, so the reason shows up in Security → Action Log next to the existing export event.
   - **Alerts & Reports**: scheduled CSV/XLSX reports supply `Scheduled report: <name> (id=<id>)` automatically, so they keep working with the flag on.
   
   When the flag is off nothing changes: no dialog, no extra parameter, no new 400, and the flag-off export paths stay fully synchronous.
   
   ### New or Changed Public Interfaces
   
   - New feature flag `REQUIRE_DOWNLOAD_REASON`.
   - New optional query parameter `download_reason` on `GET/POST /api/v1/chart/data`, `GET /api/v1/sqllab/export/<client_id>/`, `POST /api/v1/sqllab/export_streaming/`, `POST /api/v1/dashboard/<pk>/export_xlsx/`. Required (400 otherwise) only when the flag is on. Documented in the OpenAPI spec and `UPDATING.md`.
   - No new models, tables, or React public components. One new util module on each side (`superset/utils/download_reason.py`, `src/utils/downloadReason.tsx`).
   
   ### New dependencies
   
   None.
   
   ### Migration Plan and Compatibility
   
   No database migration. No stored URL changes. API clients that call the export endpoints must pass `download_reason` once an operator enables the flag; this is called out in `UPDATING.md`.
   
   ### Rejected Alternatives
   
   - **A config key instead of a feature flag.** The frontend has to know whether to show the prompt. Feature flags already reach the browser through the bootstrap payload with a typed `FeatureFlag` enum; a config key would need the same boolean plumbed through common bootstrap data plus a frontend type. `GRANULAR_EXPORT_CONTROLS` is existing precedent for gating export behaviour with a flag. Open to switching if the community prefers a config key.
   - **A dedicated `download_reasons` table.** Adds a model, migration, DAO and API for data the event logger already captures. The Action Log is where operators look for export events, so the reason belongs next to them.
   - **Making the reason always required.** Most deployments do not need it, and it adds friction to every export. Opt-in only.
   - **Logging rejected (no-reason) attempts.** A rejected request downloads nothing, and the UI always sends a reason when the flag is on, so a 400 only comes from an API client that was not updated. Left as-is; if wanted, that is a change to `log_context` itself, not to this feature.
   


-- 
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.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: [I] [SIP-230] Proposal for an optional download reason on data exports [superset]

Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on issue #44512:
URL: https://github.com/apache/superset/issues/44512#issuecomment-5969541615

   Curious to hear if @michael-s-molina thinks this should be a config or feature flag. I thought we were trying to reduce overuse of feature flags and this feels to me like a config setting?
   
   Otherwise I like it as is and would vote+1 on it. Thanks for the SIP!


-- 
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: [I] [SIP-230] Proposal for an optional download reason on data exports [superset]

Posted by "seunggabi (via GitHub)" <gi...@apache.org>.
seunggabi commented on issue #44512:
URL: https://github.com/apache/superset/issues/44512#issuecomment-5812234432

   @kadubhumika https://github.com/apache/superset/pull/44499 ing~
   


-- 
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: [I] [SIP-230] Proposal for an optional download reason on data exports [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on issue #44512:
URL: https://github.com/apache/superset/issues/44512#issuecomment-6297448645

   Relaying from the email thread:
   
   > 1. Feature flag vs. config key for REQUIRE_DOWNLOAD_REASON.
   
   I vote config. If this isn’t a feature rollout path, or something that might be on the road to deprecation, it should be considered config. 
   
   For a random historical sidebar, there was some debate about boolean config items getting lumped into feature flags at one point, but I think we’ve largely eschewed that idea, and see config as the right place for “settings for permanent features” and feature flags being more for feature release/removal and trying to trim down that list over time since it multiplies complexity of releases/testing.
   
   > 2. Recording the reason in the existing Action Log entry (no new table) - any concerns?
   
   I think this would be preferred in my book… this reason sounds like it IS an action log, so no need to add the maintenance footprint and testing/migration complexities of a new table.
   
   > 3. Anything missing from the scope (Explore, dashboard charts,dashboard-wide Excel, SQL Lab, scheduled reports)?
   
   Not sure… there may be any number of places in the future to add a “reason” input and store it in the same way. I think the main thing would be to make sure this is as centralized and “DRY” as possible so it can be cleanly reused with a minimal code footprint wherever it’s needed in the future. Which could go beyond this download reasoning to other reasons (e.g. installing an Extension, or punching a hole in the CSP so an Extension can reach an external service).
   
   > If there are no objections by Oct 28, I'll open the [VOTE] thread.
   
   Sounds like a plan! If the above are in place, you’ll likely get my +1 ;)


-- 
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: [I] [SIP-230] Proposal for an optional download reason on data exports [superset]

Posted by "kadubhumika (via GitHub)" <gi...@apache.org>.
kadubhumika commented on issue #44512:
URL: https://github.com/apache/superset/issues/44512#issuecomment-5808909558

   Hi team! I am an intermediate backend engineer with strong experience in Python, SQLAlchemy, and PostgreSQL optimization (including connection pooling and indexing). I would love to work on this issue and raise a PR to fix it. Could you please assign this to me?


-- 
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: [I] [SIP-230] Proposal for an optional download reason on data exports [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on issue #44512:
URL: https://github.com/apache/superset/issues/44512#issuecomment-5801822029

   Thanks @seunggabi, this is a well-written SIP. Looks like @sadpandajoe already numbered it SIP-230, so I'm just adding the `sip` label. Next step is a [DISCUSS] thread on the `dev@` mailing list (you'll need to be subscribed for it to post), and #44499 stays under `hold:sip!` until the vote. Holler if you want a hand navigating any of that.


-- 
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: [I] [SIP-230] Proposal for an optional download reason on data exports [superset]

Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on issue #44512:
URL: https://github.com/apache/superset/issues/44512#issuecomment-6298160949

   Let's go with config, then.


-- 
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: [I] [SIP-230] Proposal for an optional download reason on data exports [superset]

Posted by "kadubhumika (via GitHub)" <gi...@apache.org>.
kadubhumika commented on issue #44512:
URL: https://github.com/apache/superset/issues/44512#issuecomment-5813990669

   Okay @seunggabi 


-- 
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: [I] [SIP-230] Proposal for an optional download reason on data exports [superset]

Posted by "seunggabi (via GitHub)" <gi...@apache.org>.
seunggabi commented on issue #44512:
URL: https://github.com/apache/superset/issues/44512#issuecomment-5986891312

   Thanks for the review and the +1 @sfirke!
   
   On flag vs. config: I agree this is closer to a long-lived deployment policy than a rollout toggle, so a config key is a reasonable fit. I went with a feature flag only because of the existing frontend plumbing: flags already reach the browser through the bootstrap payload with a typed `FeatureFlag` enum, and `GRANULAR_EXPORT_CONTROLS` is a precedent for gating export behaviour that way.
   
   Switching is cheap, though. A `REQUIRE_DOWNLOAD_REASON` config key would be added to `FRONTEND_CONF_KEYS` in `superset/views/base.py` (with a matching frontend type) and the backend would read `app.config` instead of `is_feature_enabled`. The behaviour is unchanged: default `False`, dialog + `400` only when on, Alerts & Reports supplying their own reason.
   
   @michael-s-molina if you prefer the config key, I'm happy to update #44499 and the SIP text accordingly. Otherwise I'll keep the flag as proposed. Either way works for me.


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