You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@superset.apache.org by "kgabryje (via GitHub)" <gi...@apache.org> on 2023/08/10 14:25:11 UTC

[GitHub] [superset] kgabryje opened a new issue, #24948: [SIP] Proposal for Adding Playwright as an Alternative for Selenium to Enable deck.gl Charts in Reports

kgabryje opened a new issue, #24948:
URL: https://github.com/apache/superset/issues/24948

### Motivation

Currently, Superset uses Selenium for rendering charts in reports and in thumbnails. However, this approach has proven to be problematic when dealing with [deck.gl](http://deck.gl/) charts. The current Selenium-based approach fails to handle these charts properly, resulting in blank visualizations in the reports. To address this issue and provide a more reliable solution for rendering [deck.gl](http://deck.gl/) charts, we propose integrating [Playwright](https://github.com/microsoft/playwright) as an alternative to Selenium.

### Proposed Change

The proposed change involves introducing Playwright for rendering charts in Superset reports. Playwright is an open-source library for automating web browsers, similar to Selenium but with better support for modern browser features and improved performance. By using Playwright, we aim to provide a more stable and accurate chart rendering experience in Superset reports, especially for [deck.gl](http://deck.gl/) charts.

We wrote a POC that proved that the underlying problem, which is inability to render WebGL images (which deck.gl charts are based on), does not occur when running reports with Playwright. The screenshots below present the contents of email reports of dashboard containing deck.gl charts when using Selenium (before) and Playwright (after).

Before (Selenium):

After (Playwright):

![image](https://github.com/apache/superset/assets/15073128/3917d759-1344-467f-ba32-cc8e639a4496)

Since configuring Playwright requires installing additional dependencies, in order to prevent breaking changes in existing deployments we propose to put the new flow behind a feature flag `PLAYWRIGHT_REPORTS_AND_THUMBNAILS` while supporting Selenium at the same time. Users who do not enable the new feature flag will be unaffected by the proposed changes.

### New or Changed Public Interfaces

There will be no changes to existing public interfaces in Superset as part of this feature addition.

### New dependencies

The proposed feature will require the addition of the Playwright library as a new Python dependency. Playwright is actively maintained, with Apache 2.0 license.

### Migration Plan and Compatibility

The migration to Playwright as the alternative to Selenium will not require any database migrations or updates to stored URLs. Existing reports using [deck.gl](http://deck.gl/) charts should continue to work as expected.

In order to avoid introducing a potential breaking change in existing deployments, Playwright will only be used if `PLAYWRIGHT_REPORTS_AND_THUMBNAILS` is enabled. Otherwise, the reports will be run by Selenium.

### Rejected Alternatives

1. Continue using Selenium with geckodriver (Firefox): This approach was rejected due to the limitations of geckodriver, which can't render WebGL visualizations in headless mode.
2. Continue using Selenium, change the driver to Chrome: This approach was rejected due to problems with taking fullpage screenshots.

--
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.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

[GitHub] [superset] kgabryje commented on issue #24948: [SIP-98] Proposal for Adding Playwright as an Alternative for Selenium to Enable deck.gl Charts in Reports

Posted by "kgabryje (via GitHub)" <gi...@apache.org>.

kgabryje commented on issue #24948:
URL: https://github.com/apache/superset/issues/24948#issuecomment-1712793167

   SIP implemented by https://github.com/apache/superset/pull/25247. Thanks to everyone who participated in the discussion!


-- 
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

[GitHub] [superset] kgabryje commented on issue #24948: [SIP] Proposal for Adding Playwright as an Alternative for Selenium to Enable deck.gl Charts in Reports

Posted by "kgabryje (via GitHub)" <gi...@apache.org>.

kgabryje commented on issue #24948:
URL: https://github.com/apache/superset/issues/24948#issuecomment-1677582692

   > Is the intention here to rename the plugins and remove `legacy` from their names or create new ones? What will be the relationship with ECharts GL charts? Do you think we should write a specific SIP about this topic?
   
   I don't think the plugins are _fundamentally_ bad - it's a number of smaller issues that make the experience worse than newer plugins. So in my opinion, we should rather focus on fixing, refactoring and improving their usability rather then starting from scratch. If we do that + eventually migrate to API V1, we could drop the "legacy" prefix.
   As for the SIP - I think migrating to another library or completely changing the architecture (which I think works well in our current plugins) would warrant a SIP, which is not the case here.


-- 
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

[GitHub] [superset] eschutho commented on issue #24948: [SIP-98] Proposal for Adding Playwright as an Alternative for Selenium to Enable deck.gl Charts in Reports

Posted by "eschutho (via GitHub)" <gi...@apache.org>.

eschutho commented on issue #24948:
URL: https://github.com/apache/superset/issues/24948#issuecomment-1690356060

   > > @kgabryje do you think there's merit in first raising this problem as a [Selenium GitHub issue](https://github.com/SeleniumHQ/selenium/issues)? Their community may be aware of a workaround and/or could potentially gauge the scale/complexity of the problem. Again the more relevant data we have the easier to weigh up the various pros and cons of the proposal.
   > 
   > Our version of Selenium (3.141.0) is from 2018 (the Chromium driver was equally ancient) and probably the first thing the community would say is to upgrade to the latest version and see if it solves the problem. I do not know how much effort this upgrade would require, but I think it might be a good time to introduce a newer tool that covers our use cases, while offering a better developer experience (easier to write and read) than Selenium.
   > 
   > I see the drawback of maintaining two libraries/flows, but this would be only temporary until we're ready to make a breaking change and stick to 1 library only.
   
   @john-bodley I also spent a little time looking into the selenium github issues for previous questions on the topic. When most people were looking for full-page screenshots, they would direct them to use the Chrome driver and when they needed  webgl support, they would direct them to use the Firefox driver. I couldn't find any use cases for either issue where it could be solved by either driver. 


-- 
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

[GitHub] [superset] john-bodley commented on issue #24948: [SIP] Proposal for Adding Playwright as an Alternative for Selenium to Enable deck.gl Charts in Reports

Posted by "john-bodley (via GitHub)" <gi...@apache.org>.

john-bodley commented on issue #24948:
URL: https://github.com/apache/superset/issues/24948#issuecomment-1673433191

   Thanks @kgabryje for putting this SIP together. Would it be possible to expand further on the rejected alternatives? Specifically are these mentioned issues tracked in the Selenium project as known issues, and if so is their an ETA in terms of when they might be resolved?
   
   Granted it seems like Playwright circumvents the issue currently experienced with Selenium (using either the Firefox or Chrome driver), but it does come at a cost in terms of introducing (and thus supporting) another framework.
   
   The TL;DR is there’s always pros and cons with any approach and thus would be helpful to fully understand the situation before voting on this SIP.


-- 
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

[GitHub] [superset] michael-s-molina commented on issue #24948: [SIP] Proposal for Adding Playwright as an Alternative for Selenium to Enable deck.gl Charts in Reports

Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.

michael-s-molina commented on issue #24948:
URL: https://github.com/apache/superset/issues/24948#issuecomment-1673766503

> Most (all?) geospacial visualization libraries are based on WebGL, so even if we did migrate from deck.gl to another lib, we'd face the same issue.

Fair point.

> As for replacing deck.gl with another framework - I don't think it's a good way forward. Deck.gl is a well-maintained library with a big community. Our chart plugins are buggy, but I wouldn't put the blame (or at least not all of it) on the library itself - the plugins haven't been properly maintained for years and with some effort we can bring them to the same level of quality as the newer plugins.
I started that effort by https://github.com/apache/superset/pull/24933 in order to make them easier to maintain. More of those are coming - refactoring from class to functional components, implementing best practices as described in deck.gl docs, etc.

Is the intention here to rename the plugins and remove `legacy` from their names or create new ones? What will be the relationship with ECharts GL charts? Do you think we should write a specific SIP about this topic?

--
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

Re: [I] [SIP-98] Proposal for Adding Playwright as an Alternative for Selenium to Enable deck.gl Charts in Reports [superset]

Posted by "iamsaso (via GitHub)" <gi...@apache.org>.

iamsaso commented on issue #24948:
URL: https://github.com/apache/superset/issues/24948#issuecomment-1887943937

   @kgabryje is there a reason you didn't include `full_page=True` parameter for playwright in your 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

[GitHub] [superset] michael-s-molina closed issue #24948: [SIP-98] Proposal for Adding Playwright as an Alternative for Selenium to Enable deck.gl Charts in Reports

Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.

michael-s-molina closed issue #24948: [SIP-98] Proposal for Adding Playwright as an Alternative for Selenium to Enable deck.gl Charts in Reports
URL: https://github.com/apache/superset/issues/24948


-- 
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

[GitHub] [superset] michael-s-molina commented on issue #24948: [SIP] Proposal for Adding Playwright as an Alternative for Selenium to Enable deck.gl Charts in Reports

Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.

michael-s-molina commented on issue #24948:
URL: https://github.com/apache/superset/issues/24948#issuecomment-1673547385

   Thank you @kgabryje for writing this SIP. I think it's great when we take the time to write SIPs and have the opportunity to discuss big changes before they are merged to master.
   
   Given that deck.gl charts are considered legacy charts that will be removed in the future, did we consider the effort/benefit of migrating them vs introducing another framework that we'll need to support? I don't know if we already have replacements for them, but if we do and the cost is not significantly higher, we could end up in a better state with just one framework and less legacy charts.


-- 
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

[GitHub] [superset] villebro commented on issue #24948: [SIP-98] Proposal for Adding Playwright as an Alternative for Selenium to Enable deck.gl Charts in Reports

Posted by "villebro (via GitHub)" <gi...@apache.org>.

villebro commented on issue #24948:
URL: https://github.com/apache/superset/issues/24948#issuecomment-1705495099

   My quick thoughts on the discussed topics:
   
   - While ECharts does have similar geospatial functionality as DeckGL, I don't think it's is designed to be a replacement for DeckGL. As we're already doing a fair bit of advanced layering in our DeckGL plugins, I doubt we'd be able to achieve feature parity on our geospatial charts with ECharts (or any other library for that matter).
   - For solving this particular issue, Playwright does seem to be a great alternative that solves our set of minimum requirements. In the interim supporting both is probably the best alternative, but in the long run we should remove support for Selenium in a future major version to reduce maintenance burden if no new feature parity issues arise.


-- 
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

[GitHub] [superset] betodealmeida commented on issue #24948: [SIP] Proposal for Adding Playwright as an Alternative for Selenium to Enable deck.gl Charts in Reports

Posted by "betodealmeida (via GitHub)" <gi...@apache.org>.

betodealmeida commented on issue #24948:
URL: https://github.com/apache/superset/issues/24948#issuecomment-1681228705

   I think that regardless of the state of the legacy deck.gl visualizations or the path we take forward (improve them, replace them with ECharts GL, etc.), **we do need to support WebGL in Superset**, since not only it's used everywhere in geoviz but also in custom visualizations that handle huge quantities of data. Personally I would love to see the deck.gl visualizations being improved and brought back to tier 1, but I suggest we have that discussion in a separate channel (or SIP), and focus on what solution we're going to use for WebGL.
   
   While I think it might be worth trying the newer version of Selenium (4.11.2 vs the current 3.141.0), I think Playwright has other advantages, since it's much easier to install and run and has a more modern API (with optional support for async). Taking a screenshot with Playwright is incredible simple, since you don't need to worry about all the webdriver options:
   
   ```bash
   pip install playwright
   playwright install
   ```
   
   ```python
   import asyncio
   from playwright.async_api import async_playwright
   
   async def main():
       async with async_playwright() as p:
           browser = await p.chromium.launch()
           page = await browser.new_page()
           await page.goto("https://superset.apache.org")
           print(await page.title())
           await page.screenshot(path="screenshot.png", full_page=True)
           await browser.close()
   
   asyncio.run(main())
   ```


-- 
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

[GitHub] [superset] kgabryje commented on issue #24948: [SIP] Proposal for Adding Playwright as an Alternative for Selenium to Enable deck.gl Charts in Reports

Posted by "kgabryje (via GitHub)" <gi...@apache.org>.

kgabryje commented on issue #24948:
URL: https://github.com/apache/superset/issues/24948#issuecomment-1677567104

   > @kgabryje do you think there's merit in first raising this problem as a [Selenium GitHub issue](https://github.com/SeleniumHQ/selenium/issues)? Their community may be aware of a workaround and/or could potentially gauge the scale/complexity of the problem. Again the more relevant data we have the easier to weigh up the various pros and cons of the proposal.
   
   Our version of Selenium (3.141.0) is from 2018 (the Chromium driver was equally ancient) and probably the first thing the community would say is to upgrade to the latest version and see if it solves the problem. I do not know how much effort this upgrade would require, but I think it might be a good time to introduce a newer tool that covers our use cases, while offering a better developer experience (easier to write and read) than Selenium.
   
   I see the drawback of maintaining two libraries/flows, but this would be only temporary until we're ready to make a breaking change and stick to 1 library only.


-- 
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

[GitHub] [superset] kgabryje commented on issue #24948: [SIP] Proposal for Adding Playwright as an Alternative for Selenium to Enable deck.gl Charts in Reports

Posted by "kgabryje (via GitHub)" <gi...@apache.org>.

kgabryje commented on issue #24948:
URL: https://github.com/apache/superset/issues/24948#issuecomment-1673329064

   CC @eschutho @villebro @michael-s-molina @john-bodley 


-- 
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: [I] [SIP-98] Proposal for Adding Playwright as an Alternative for Selenium to Enable deck.gl Charts in Reports [superset]

Posted by "SeanStock-THG (via GitHub)" <gi...@apache.org>.

SeanStock-THG commented on issue #24948:
URL: https://github.com/apache/superset/issues/24948#issuecomment-1956580624

   Hi @kgabryje, seem to have seen a massive increase in CPU and RAM resources on taking the screenshots. Is this expected? not sure if can be set to use a set user like we did with selnium to avoid it having to load 1000+ charts for each user. 


-- 
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: [I] [SIP-98] Proposal for Adding Playwright as an Alternative for Selenium to Enable deck.gl Charts in Reports [superset]

Posted by "kgabryje (via GitHub)" <gi...@apache.org>.

kgabryje commented on issue #24948:
URL: https://github.com/apache/superset/issues/24948#issuecomment-1888992027

   > @kgabryje is there a reason you didn't include `full_page=True` parameter for playwright in your PR?
   
   @iamsaso `full_page=True` is an argument of `.screenshot()` method when it's called on `page` object. Here we call it on `element` object, which screenshots the whole element regardless of viewport size.


-- 
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

[GitHub] [superset] kgabryje commented on issue #24948: [SIP] Proposal for Adding Playwright as an Alternative for Selenium to Enable deck.gl Charts in Reports

Posted by "kgabryje (via GitHub)" <gi...@apache.org>.

kgabryje commented on issue #24948:
URL: https://github.com/apache/superset/issues/24948#issuecomment-1677704100

   
   > Will we ignore ECharts GL? Use it to replace part of the legacy plugins? Use it to enhance our legacy plugin?
   
   Sorry, missed that question. I'm not very familiar with EchartsGL but to my understanding it's an extension of Echarts that provides geospatial visualisations, so it has the same use case as deck.gl. What do you have in mind about "replacing" or "enhancing" our plugins? Are there visualisations that are available in EchartsGL but not in deck.gl?
   @villebro I recall we were chatting about EchartsGL vs deck.gl some time ago if you want to chime in 😄 
   


-- 
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

[GitHub] [superset] michael-s-molina commented on issue #24948: [SIP] Proposal for Adding Playwright as an Alternative for Selenium to Enable deck.gl Charts in Reports

Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.

michael-s-molina commented on issue #24948:
URL: https://github.com/apache/superset/issues/24948#issuecomment-1677908855

> What do you have in mind about "replacing" or "enhancing" our plugins?

I'm thinking about the effort to adopt ECharts as our official visualization library. If ECharts GL provide the same features as DeckGL, then maybe we should invest in migrating to ECharts GL instead of improving the DeckGL plugin. If that's not the case, maybe we can still enhance our chart library by enabling some of the ECharts GL charts which would live alongside DeckGL.

> Are there visualisations that are available in EchartsGL but not in deck.gl?

I don't know but I think this investigation is kind of a prerequisite before we decide to revamp the legacy plugin. The question we should ask is: Does ECharts supports our GL requirements with their charts? If yes, let's migrate. If no, let's improve the DeckGL plugin and remove the legacy prefix.

--
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

[GitHub] [superset] michael-s-molina commented on issue #24948: [SIP] Proposal for Adding Playwright as an Alternative for Selenium to Enable deck.gl Charts in Reports

Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.

michael-s-molina commented on issue #24948:
URL: https://github.com/apache/superset/issues/24948#issuecomment-1677681730

   Thanks for the responses @kgabryje. Regarding: 
   
   > What will be the relationship with ECharts GL charts?
   
   - Will we ignore ECharts GL? Use it to replace part of the legacy plugins? Use it to enhance our legacy plugin?


-- 
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

[GitHub] [superset] john-bodley commented on issue #24948: [SIP] Proposal for Adding Playwright as an Alternative for Selenium to Enable deck.gl Charts in Reports

Posted by "john-bodley (via GitHub)" <gi...@apache.org>.

john-bodley commented on issue #24948:
URL: https://github.com/apache/superset/issues/24948#issuecomment-1673739849

   @kgabryje do you think there's merit in first raising this problem as a [Selenium GitHub issue](https://github.com/SeleniumHQ/selenium/issues)? Their community may be aware of a workaround and/or could potentially gauge the scale/complexity of the problem. Again the more relevant data we have the easier to weigh up the various pros and cons of the proposal.


-- 
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

[GitHub] [superset] kgabryje commented on issue #24948: [SIP] Proposal for Adding Playwright as an Alternative for Selenium to Enable deck.gl Charts in Reports

Posted by "kgabryje (via GitHub)" <gi...@apache.org>.

kgabryje commented on issue #24948:
URL: https://github.com/apache/superset/issues/24948#issuecomment-1673719409

Thanks for comments!

@john-bodley Historically we switched the driver from Chrome to Firefox as a workaround for screenshots in the reports being cut off. I couldn't find a relevant issue in Selenium's repo, neither recent nor from the time when that change was made in Superset. Firefox on the other hand does not support rendering WebGL in headless mode (https://bugzilla.mozilla.org/show_bug.cgi?id=1375585)
As for Playwright running Chromium, it does seem to cover all bases and I didn't get it to fail in any common scenario - screenshot of a whole dashboard (regardless of the layout) or of a specific element, and most importantly, it does render WebGL.
I agree that having 2 libraries that essentially do the same thing is a big drawback. However, if Playwright turns out to be a success in a long run, I think we might consider fully replacing Selenium and doing a switch in a major release.

@michael-s-molina Most (all?) geospacial visualization libraries are based on WebGL, so even if we did migrate from deck.gl to another lib, we'd face the same issue.
As for replacing deck.gl with another framework - I don't think it's a good way forward. Deck.gl is a well-maintained library with a big community. Our chart plugins are buggy, but I wouldn't put the blame (or at least not all of it) on the library itself - the plugins haven't been properly maintained for years and with some effort we can bring them to the same level of quality as the newer plugins.
I started that effort by [refactoring all files to Typescript](https://github.com/apache/superset/pull/24933) in order to make them easier to maintain. More of those are coming - refactoring from class to functional components, implementing best practices as described in deck.gl docs, etc.

--
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

[GitHub] [superset] JohnDietrich-Pepper commented on issue #24948: [SIP] Proposal for Adding Playwright as an Alternative for Selenium to Enable deck.gl Charts in Reports

Posted by "JohnDietrich-Pepper (via GitHub)" <gi...@apache.org>.

JohnDietrich-Pepper commented on issue #24948:
URL: https://github.com/apache/superset/issues/24948#issuecomment-1680833583

   ECharts GL does not support the same functionality as Deck.gl charts at this point in time particularly the flexibility to render things like contour maps that has a pending PR on Superset.  Compare
   https://echarts.apache.org/examples/en/index.html#chart-type-map
   https://[deck.gl](https://deck.gl/examples/arc-layer)/examples/arc-layer
   
   Also compared to Deck.gl the ECharts GL seems significantly slower on render on a pretty fast machine (6800HS / 4070) vs. Deck.gl in the examples (hard to compare as examples are a bit different).
   
   Finally, there seems to be interest in actually developing deck.gl charts (heatmap and contour being recent examples) whereas there is no pending PR to add ECharts GL.


-- 
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

[GitHub] [superset] michael-s-molina commented on issue #24948: [SIP-98] Proposal for Adding Playwright as an Alternative for Selenium to Enable deck.gl Charts in Reports

Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.

michael-s-molina commented on issue #24948:
URL: https://github.com/apache/superset/issues/24948#issuecomment-1713706797

   Closing the issue as it was approved.


-- 
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