EnxDev opened a new pull request, #44425:
URL: https://github.com/apache/superset/pull/44425
### SUMMARY
Exporting a chart or dashboard to CSV/Excel fails when the row limit is
100,000 or higher and the dataset sits on a BigQuery connection that
authenticates with OAuth2. The file downloads, but it contains an error marker
instead of any rows.
At or above `CSV_STREAMING_ROW_THRESHOLD` (100k by default) the export
switches from the buffered path to the streaming one. The streaming path hands
Werkzeug a generator and returns, so the query actually runs *after* Flask has
torn down the request context. To cope with that, the command snapshots
`flask.g` and replays it inside a fresh app context.
The snapshot copied `g.user` as-is. Flask-AppBuilder sets that to
Flask-Login's `current_user`, which is a `LocalProxy` that checks
`has_request_context()` and resolves to `None` once the request is over. So the
generator ended up running as nobody at all. The guard in
`Database._get_sqla_engine` (`hasattr(g.user, "id")`) is then `False`, the
engine is built with `access_token=None`, and the user's OAuth2 token never
reaches BigQuery. The connection fails, the generator's catch-all writes a
`__STREAM_ERROR__` marker into the response body, and that marker is what ends
up in the downloaded file.
That also explains the two things that made this look odd:
- it only reproduces at exactly 100k rows, because below the threshold the
buffered path never leaves the request context;
- it only reproduces with OAuth2, because service account credentials live
on the connection itself and don't depend on who is asking.
The fix resolves request-bound proxies to the concrete object they point at
while the request context is still alive, so the generator sees the same acting
user as every other place that acquires an engine. Impersonation and
`start_oauth2_dance` were losing the user for the same reason and are fixed
along with it.
Worth flagging: there was already a test claiming to pin this behaviour, but
it set `g.user` to a plain `Mock`, which survives being copied, so it passed
while production broke. The new test uses a real `LocalProxy` across a genuine
request boundary and fails without the fix.
One thing this does not solve: if a user has no token at all, or no refresh
token, the streaming path still cannot prompt them to re-authenticate, because
the response headers have already gone out by the time we find out. Handling
that means checking auth before streaming starts, which felt like a separate
change rather than something to bundle in here.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable, backend only.
### TESTING INSTRUCTIONS
Manually:
1. Create a BigQuery database connection using OAuth2, and a dataset on top
of it.
2. Build a simple table chart on that dataset and set the row limit to
100000.
3. Download the chart as CSV or Excel.
Before this change the file contains a `__STREAM_ERROR__` marker and no
rows. After it the export completes normally. Anything below 100000 rows was
already working and still does.
Automated:
```bash
pytest tests/unit_tests/commands/chart/streaming_export_command_test.py \
tests/unit_tests/commands/sql_lab/streaming_export_command_test.py
```
### 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: [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]
