rusackas commented on PR #34525: URL: https://github.com/apache/superset/pull/34525#issuecomment-3857228697
## PR Rebased and Improved This PR has been rebased onto latest master with the following improvements: ### Conflict Resolution - **`superset/tasks/cache.py`** - Merged WebDriver approach with current codebase - **`superset/utils/screenshots.py`** - Integrated with new `PLAYWRIGHT_AVAILABLE` fallback logic - **`superset/utils/webdriver.py`** - Merged persistent driver approach with refactored helper methods (`_create_firefox_driver`, `_create_chrome_driver`, `_normalize_timeout_values`) ### Improvements 1. **Error handling** - Added check for when `SUPERSET_CACHE_WARMUP_USER` doesn't exist 2. **Resource cleanup** - Added explicit `try/finally` with `del wd` to ensure WebDriver cleanup (addresses the Korbit bot review about potential resource leaks) 3. **Code cleanup** - Removed unused `TypedDict` classes --- ### Architectural Trade-offs for Reviewers This PR takes a **fundamentally different approach** than the current implementation: | Aspect | Current (API-based) | This PR (WebDriver-based) | |--------|---------------------|---------------------------| | **Mechanism** | HTTP calls to `/api/v1/chart/warm_up_cache` | Selenium renders full dashboards | | **Auth** | `MachineAuthProvider` cookies | WebDriver with authenticated session | | **Config** | `CACHE_WARMUP_EXECUTORS` (per-chart) | `SUPERSET_CACHE_WARMUP_USER` (single user) | | **Scope** | Individual charts | Entire dashboards | | **Dependencies** | None (HTTP only) | Selenium + browser in Celery worker | **Pros of WebDriver approach:** - ✅ Simulates real user behavior exactly - ✅ Caches populated as users would experience them - ✅ Simpler configuration (single user vs. executor mapping) - ✅ Dashboard context preserved (filters, cross-filtering, etc.) **Cons of WebDriver approach:** - ⚠️ Requires browser (Chrome/Firefox) available in Celery worker environment - ⚠️ More resource-intensive than API calls - ⚠️ Slower (full page render vs. API call) This is the same approach used for thumbnail generation, so environments already configured for thumbnails will work without additional setup. -- 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]
