mikebridge opened a new pull request, #45987: URL: https://github.com/apache/superset/pull/45987
### SUMMARY **TL;DR:** Release the unit-test session factory's sessions at teardown so dashboard tests do not retain version-capture state. **Background:** #45364 addressed the per-module app leak reported in #45362. The dashboard DAO unit tests still retained a substantial amount of memory after completing. **Problem:** The shared `get_session` fixture creates sessions without rolling them back or closing them. Flushed, uncommitted versioned objects leave sessions reachable through Continuum's capture bookkeeping, even after pytest removes the session mocks. **Changes:** Track rollback and close callbacks for every session the factory creates, including multiple calls within one test. Run them at fixture teardown. Add weak-reference regression checks after function-scoped fixtures have finished. No production versioning behavior changes, and no CI memory-report feature is included. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF No UI change. Local fresh-process measurements of `dao/dashboard_test.py`, from the first test's fixture setup to module completion plus garbage collection: | Measure | Before | After | | --- | --- | --- | | RSS without tracemalloc | 407.89 → 565.70 MiB (+157.81) | 397.20 → 456.94 MiB (+59.73) | | Traced live Python allocations, separate runs | 259.10 → 306.74 MiB (+47.64) | 259.11 → 278.18 MiB (+19.07) | | Live SQLAlchemy sessions after module completion | 72 | 0 | | Continuum units of work / session-connection entries after replacing the app and collecting | 36 / 36 | 0 / 0 | These are macOS/Python 3.12 measurements using SQLAlchemy 2.0.52 (CI pins 2.0.55), not CI peak-memory estimates. RSS includes allocator/cache effects; tracing has its own overhead. Residual allocations remain, largely in SQLAlchemy SQL construction/compiler/type caches; this does not claim all process memory growth is eliminated. ### TESTING INSTRUCTIONS - Run `python -m pytest tests/unit_tests/dao -q`: 235 passed locally. - Run changed-file pre-commit hooks, including MyPy. - The new tests create one and two sessions, flush real versioned dashboards, and check weak references after the fixture teardown. Without the cleanup callbacks, the final regression reports `2 passed, 1 error` at teardown because sessions remain reachable. With cleanup, both cases pass. - Full-suite CI remains necessary because the fixture is shared; no full local suite or service-backed tests were run. ### ADDITIONAL INFORMATION - [x] Has associated issue: follow-up to #45362 and #45364 - [ ] 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]
