You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2026/04/27 17:33:31 UTC
[PR] fix(datasets): isolate filter state to fix concurrent /dataset race [superset]
rusackas opened a new pull request, #39685:
URL: https://github.com/apache/superset/pull/39685
## Summary
Adopts #33895 (thanks @MIKEX818!) with the requested cleanups and a regression test.
`DatasetRestApi` now overrides `_handle_filters_args` so each request builds a fresh `Filters` instance. The Flask-AppBuilder default mutates a single `self._filters` shared across requests, which under concurrent traffic leaks filter state from one request into another — observed in #33828 as `GET /api/v1/dataset/?q=(filters:!((col:table_name,opr:eq,value:...)))` returning the *wrong* dataset roughly 5% of the time when ~12 parallel calls fan out.
Returning a request-scoped `Filters` keeps each call isolated.
### Changes vs. #33895
Addresses review feedback from @hainenber and @sadpandajoe:
- Drop unused `import copy`.
- Drop the duplicate `from typing import Any, Callable, Dict` line; use modern `dict[str, Any]` annotation instead.
- Pull `API_FILTERS_RIS_KEY` into the existing FAB-const import block.
- Clean up the docstring (the original had garbled paste artifacts and a dead `# self._filters.clear_filters()` comment) and explicitly note this overrides the FAB method (clarifying the question @sadpandajoe raised).
- Add a unit test (`test_handle_filters_args_returns_request_scoped_filters`) that asserts a fresh `Filters` instance is constructed per call — the property that prevents the race.
Re: @Vitor-Avila's question about whether this should live in FAB instead — that's a reasonable position, but FAB hasn't responded since June 2025 and Superset users are hitting this in production. Patching here unblocks 5.x; FAB can absorb it later.
### Before / After
Before: parallel `GET /api/v1/dataset/` filtered by different `table_name` values intermittently returns mismatched datasets.
After: each request's filter state is isolated; the race window is closed.
## Testing instructions
- [ ] `pytest tests/unit_tests/datasets/api_tests.py::test_handle_filters_args_returns_request_scoped_filters` passes.
- [ ] Reproduce from #33828: fire ~12 concurrent `GET /api/v1/dataset/?q=(filters:!((col:table_name,opr:eq,value:<unique>)))` requests with distinct `table_name` values; each response should match its query's `table_name` 100% of the time.
## Additional information
- Closes #33828
- Adopts #33895 (original author: @MIKEX818)
🤖 Generated with [Claude Code](https://claude.com/claude-code) via Custodial Engineer task #10
--
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: [PR] fix(datasets): isolate filter state to fix concurrent /dataset race [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #39685:
URL: https://github.com/apache/superset/pull/39685#issuecomment-4329246419
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/39685?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 63.67%. Comparing base ([`b791f4c`](https://app.codecov.io/gh/apache/superset/commit/b791f4c2cd2e3489572c191bda79089a2fafe010?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`b03642d`](https://app.codecov.io/gh/apache/superset/commit/b03642df1d35cf2a7b821619814d6137e98ce8fc?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 4 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #39685 +/- ##
==========================================
- Coverage 64.48% 63.67% -0.81%
==========================================
Files 2566 2566
Lines 133926 133931 +5
Branches 31096 31096
==========================================
- Hits 86357 85283 -1074
- Misses 46074 47135 +1061
- Partials 1495 1513 +18
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/39685/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/39685/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.77% <50.00%> (+<0.01%)` | :arrow_up: |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/39685/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/39685/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/39685/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.54% <50.00%> (-0.01%)` | :arrow_down: |
| [python](https://app.codecov.io/gh/apache/superset/pull/39685/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `60.07% <100.00%> (-1.80%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/39685/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `59.93% <100.00%> (+<0.01%)` | :arrow_up: |
| [unit](https://app.codecov.io/gh/apache/superset/pull/39685/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/39685?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: [PR] fix(datasets): isolate filter state to fix concurrent /dataset race [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #39685:
URL: https://github.com/apache/superset/pull/39685#issuecomment-4503279092
Done in 862b193d. Lifted `_handle_filters_args` from `DatasetRestApi` to `BaseSupersetModelRestApi` (superset/views/base_api.py), so the request-scoped fix now applies to every subclass — ChartRestApi, DashboardRestApi, SavedQueryRestApi, QueryRestApi, DatabaseRestApi, etc. — not just datasets.
The override is purely generic (no dataset-specific logic), and a grep confirmed no other superset REST API overrides `_handle_filters_args`, so zero collision risk. Docstring updated to reference the broader scope and cite #33828 for the original dataset-level repro. Test stays in `tests/unit_tests/datasets/api_tests.py` using `DatasetRestApi` as a concrete subclass to exercise the inherited behavior.
Thanks for the catch — this would've been a follow-up that nobody got to.
--
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: [PR] fix(datasets): isolate filter state to fix concurrent /dataset race [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #39685:
URL: https://github.com/apache/superset/pull/39685#issuecomment-4338784616
@betodealmeida @michael-s-molina @sadpandajoe — gentle ping on this one: CI is green and there are no open review threads. Let me know if any of you happen to have bandwidth to take a look :)
--
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: [PR] fix(datasets): isolate filter state to fix concurrent /dataset race [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #39685:
URL: https://github.com/apache/superset/pull/39685#issuecomment-4329156328
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: [PR] fix(datasets): isolate filter state to fix concurrent /dataset race [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #39685:
URL: https://github.com/apache/superset/pull/39685#issuecomment-4516692943
**Bito Automatic Review Skipped – PR Already Merged**<br/><br/>Bito scheduled an automatic review for this pull request, but the review was skipped because this PR was merged before the review could be run.<br/>No action is needed if you didn't intend to review it. To get a review, you can type `/review` in a comment and save it
--
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: [PR] fix(datasets): isolate filter state to fix concurrent /dataset race [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #39685:
URL: https://github.com/apache/superset/pull/39685#issuecomment-4503291772
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 862b193d427d824f8ca971ae992e0cc811777f3d |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a0e3a82c0966300086a6fff |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-39685--superset-docs-preview.netlify.app](https://deploy-preview-39685--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#q1YqKMrPLShRslJS0lFKKkrMS84AspNT9UsSi7N1DQ2AoimpBTn5lZ4pQHGzRINU40QLo2QDSzMzYwMDAwuzRLO0tDSlWgA) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-39685--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: [PR] fix(datasets): isolate filter state to fix concurrent /dataset race [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #39685:
URL: https://github.com/apache/superset/pull/39685
--
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