Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reports without a chart cannot be screenshot by celery worker #17245

Closed
3 tasks done
madsbuch opened this issue Oct 27, 2021 · 6 comments
Closed
3 tasks done

Reports without a chart cannot be screenshot by celery worker #17245

madsbuch opened this issue Oct 27, 2021 · 6 comments
Labels
alert-reports Namespace | Anything related to the Alert & Reports feature #bug Bug report

Comments

@madsbuch
Copy link

Scheduled reporting and alerts use a celery worker. This worker authenticates users and renders a standalone reports page using Selenium and a headless browser. Finally, this is emailed.

It appears that Selenium is set to wait until the class slice_container is available (https://github.com/apache/superset/blob/master/superset/utils/webdriver.py#L130-L134). However, it seems like this class is only added to Charts (https://github.com/apache/superset/search?q=slice_container). Th problem occurs when no charts are available on reports.

I know this is an obscure conor case that probably don't merit too much change. However, it took me some time to debug this from the error I got

[2021-10-27 11:36:30,019: WARNING/MainProcess] Selenium timed out requesting url http://superset:8088/superset/dashboard/1/?standalone=3
Traceback (most recent call last):
  File "/app/superset/utils/webdriver.py", line 134, in get_screenshot
    (By.CLASS_NAME, "slice_container")
  File "/usr/local/lib/python3.7/site-packages/selenium/webdriver/support/wait.py", line 80, in until
    raise TimeoutException(message, screen, stacktrace)
selenium.common.exceptions.TimeoutException: Message: 

Report state: Report Schedule execution failed when generating a screenshot.

How to reproduce the bug

  1. Create a dashboard without a chart
  2. Setup a scheduled report
  3. See that above-mentioned error is thrown

Expected results

To have the report screenshot and send

Actual results

Selenium times out.

Environment

(please complete the following information):

  • browser type and version: GECKODRIVER_VERSION=0.29.0
  • superset version: apache/superset:1.3.0, from dockerhub
  • python version: As bundled in the docker image
  • node.js version: As bundled in the docker image
  • any feature flags active: FEATURE_FLAGS = {"ALERT_REPORTS": True}

Checklist

Make sure to follow these steps before submitting your issue - thank you!

  • I have checked the superset logs for python stacktraces and included it here as text if there are any.
  • I have reproduced the issue with at least the latest released version of superset.
  • I have checked the issue tracker for the same issue and I haven't found one similar.

Additional context

Knowing when the page is rendered is a hard one. It might also be documentation or more verbose error handling that is the solution.

Let me know how to best contribute a solution to this, if that is an option.

@madsbuch madsbuch added the #bug Bug report label Oct 27, 2021
@villebro
Copy link
Member

@madsbuch this is indeed an unfortunate fringe case. I agree with your assessment, and you seem to have pinpointed the piece of code that would need to be modified to handle this specific case (potentially we might need to add/change some class names on the frontend components). Would you be open to taking this task on in the form of a PR? I would be happy to assist. Also, it would be interesting to hear about the use case to understand this need better (=dashboard without regular charts).

@villebro villebro added the alert-reports Namespace | Anything related to the Alert & Reports feature label Oct 29, 2021
@ensky
Copy link
Contributor

ensky commented Dec 3, 2021

it would be interesting to hear about the use case to understand this need better (=dashboard without regular charts)

We faced the same problem by trying to add an alert to a dashboard with a pure markdown document. In our scenario, this dashboard is a "help" about how to use the superset. Obviously it is not a good choice to send an alert on this "help", however unfortunately it happend to be the dashboard I used to test whether the alert system is working ;(. It took me an afternoon to figure out the problem, and had a same conclusion with @madsbuch :D

@stale
Copy link

stale bot commented Apr 17, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue .pinned to prevent stale bot from closing the issue.

@rusackas
Copy link
Member

@madsbuch / @villebro / @ensky it seems this issue has gone silent for upward of a year. I'm tempted to close it, but don't want to be dismissive here. Is this still an issue in 2.0.x, and if so, would anyone still be able/willing to open a PR?

@stale stale bot removed the inactive Inactive for >= 30 days label Jan 18, 2023
@sfirke
Copy link
Member

sfirke commented Sep 6, 2023

I validated this just now on 3.0.0rc3. Agreed that it's low priority.

@rusackas
Copy link
Member

rusackas commented Feb 1, 2024

Closing this older issue in favor of #25237
Please continue/relay any context onto that issue as needed.

As always in open-source, PRs welcome ❤️

@rusackas rusackas closed this as completed Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alert-reports Namespace | Anything related to the Alert & Reports feature #bug Bug report
Projects
None yet
Development

No branches or pull requests

5 participants