Repository navigation
feat(Reports&Alerts): remove Selenium support, require Playwright - #43028
Conversation
dosubot
Bot
added
alert-reports
Code Review Agent Run #8decf2Actionable Suggestions - 0Additional Suggestions - 4
Filtered by Review RulesBito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #43028 +/- ##
==========================================
+ Coverage 66.55% 66.77% +0.22%
==========================================
Files 2864 2862 -2
Lines 161891 161614 -277
Branches 37304 37278 -26
==========================================
+ Hits 107743 107919 +176
+ Misses 52102 51652 -450
+ Partials 2046 2043 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Can you fill out the PR description please, when you have a chance? Thanks for the PR, and happy to see this go! |
madhushreeag
changed the title
631bbe4 to
bd47178
Compare
Code Review Agent Run #ecced6Actionable Suggestions - 0Additional Suggestions - 4
Filtered by Review RulesBito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
WebDriverSelenium dead code suggestion is a false alarm, we deleted that class in this PR |
|
The flagged issue is correct. The test currently mocks the Would you like me to fetch all other comments on this PR to validate and implement fixes for them as well? |
the super patch was replaced with @patch("superset.utils.webdriver.PLAYWRIGHT_AVAILABLE", False) so the test now exercises the real availability check in WebDriverPlaywright.get_screenshot() |
|
/review |
Code Review Agent Run #a7eb20Actionable Suggestions - 0Additional Suggestions - 5
Filtered by Review RulesBito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
|
|
Thanks for this! Happy to see it go :D |
|
@madhushreeag for the PR description we could also add a note that Selenium isn't supported on |
villebro
left a comment
villebro
left a comment
There was a problem hiding this comment.
First pass comments. Basically as we're now able to do breaking changes, we should remove any code that becomes dead/a stub after the removal.
Code Review Agent Run #91380bActionable Suggestions - 0Additional Suggestions - 3
Filtered by Review RulesBito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
|
villebro
left a comment
villebro
left a comment
There was a problem hiding this comment.
Awesome work, thanks @madhushreeag !
github-project-automation
Bot
moved this from Work in Progress
to Lazy Consensus Reached
in Apache Superset 7.0
villebro
moved this from Lazy Consensus Reached
to Work in Progress
in Apache Superset 7.0
|
Bito Automatic Review Skipped – PR Already Merged |
Nothing has read SCREENSHOT_LOCATE_WAIT since Selenium support was removed in apache#43028; Playwright element waits use SCREENSHOT_PLAYWRIGHT_DEFAULT_TIMEOUT or the report execution deadline. Remove the key, the unused _screenshot_locate_wait attribute, its test config entries and the docs example that suggested tuning it.
Selenium support was removed in apache#43028, so Playwright with Chromium is the only screenshot backend as of 7.0.0. Update the unversioned admin and developer docs that still described Selenium, Firefox/geckodriver, WEBDRIVER_TYPE, WEBDRIVER_CONFIGURATION and the removed PLAYWRIGHT_REPORTS_AND_THUMBNAILS feature flag, and fix the WEBDRIVER_AUTH_FUNC example to use a Playwright BrowserContext.
…nfig Screenshots use only Playwright with Chromium since Selenium support was removed in apache#43028, so the Firefox browser installed by the INCLUDE_FIREFOX build arg is never used. Remove the arg from the Dockerfile, both docker-compose files and the docs that describe it. Also remove two settings nothing reads: EMAIL_PAGE_RENDER_WAIT, unused since apache#19261, and ENABLE_PLAYWRIGHT in docker/.env, unused since
…nfig Screenshots use only Playwright with Chromium since Selenium support was removed in apache#43028, so the Firefox browser installed by the INCLUDE_FIREFOX build arg is never used. Remove the arg from the Dockerfile, both docker-compose files and the docs that describe it. Also remove two settings nothing reads: EMAIL_PAGE_RENDER_WAIT, unused since apache#19261, and ENABLE_PLAYWRIGHT in docker/.env, unused since
3 participants
SUMMARY
Removes all Selenium support from Superset's screenshot, thumbnail, and report pipeline. Playwright is now the sole WebDriver backend.
Why
Selenium has no official support for arm64 (Apple Silicon, AWS Graviton, etc.) which is an increasingly common deployment target. Playwright provides arm64 support.
The codebase has been incrementally migrating toward Playwright via the PLAYWRIGHT_REPORTS_AND_THUMBNAILS feature flag. That migration is complete. This PR removes the old Selenium code path entirely, eliminating a dual-code maintenance burden and enabling Playwright's superior WebGL/DeckGL screenshot support unconditionally.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — internal infrastructure change, no UI impact.
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION