-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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
[SIP-3] Scheduled email reports for Slices / Dashboards #5294
Conversation
1e4a0df
to
15d1bd9
Compare
Codecov Report
@@ Coverage Diff @@
## master #5294 +/- ##
==========================================
+ Coverage 73.31% 73.47% +0.16%
==========================================
Files 67 72 +5
Lines 9589 9979 +390
==========================================
+ Hits 7030 7332 +302
- Misses 2559 2647 +88
Continue to review full report at Codecov.
|
@mistercrunch I had tagged this PR as Is there a group / set of reviewers I should ping for review? The |
@mistercrunch thank you for fixing the flaky test. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did a quick first pass on reading the code, looks high quality. It's a large feature and will require thorough review as committers will have to maintain this moving forward.
I'm thinking we should have a feature flag to allow hiding the feature if it's not configured properly.
superset/config.py
Outdated
) | ||
CELERY_RESULT_BACKEND = 'db+sqlite:///celery_results.sqlite' | ||
CELERYD_LOG_LEVEL = 'DEBUG' | ||
CELERYD_PREFETCH_MULTIPLIER = 10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why 10? 1 may be better. Personally I prefer leaving the messages in the queue instead of prefetching on a potentially busy worker
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree on 1
- I believe this was some of my testing code which made it's way through.
superset/models/schedules.py
Outdated
elif report_type == ScheduleType.slice.value: | ||
return SliceEmailSchedule | ||
|
||
return None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's implicit in python, no need for this line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed.
@mistercrunch thank you for the review. I agree on hiding it behind a configuration. Shall I introduce a global config variable If disabled
Please do let me know and I will make the changes. As regards to maintenance, I will continue to support it. This feature is important for my company (https://github.com/Affirm/) and we will rely heavily on it. I am also planning to have my team start adding more features to superset soon. |
@mistercrunch I did consider the idea of URLs for the image and embedding it within mails. However, I decided against it for the following reasons
I however do like the embedded URL approach to prevent accidental forwards of internal reports to external folks :-) I agree on having a both approaches. The current approach is easy to extend.
I can work on this in a future ticket (this issue - #623) |
@@ -446,6 +446,9 @@ class CeleryConfig(object): | |||
PLANNED_V2_AUTO_CONVERT_DATE = None # e.g. '2018-06-16' | |||
V2_FEEDBACK_URL = None # e.g., 'https://goo.gl/forms/...' | |||
|
|||
# Enable / disable scheduled email reports |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mistercrunch config to hide the feature.
Cool. We'll take the time to review the current feature while keeping in mind we'll eventually support both. |
@mahendra hi, I'd like to try the feature about email with your code. can I get the source code and deploy it on my machine? Thanks |
Hi @YHGui, Which future feature do you want to see the code for? |
@mahendra soryy, it's a typo. I'd like to try the email feature. |
@YHGui - oh, you want to try this PR on your machine? The branch lives here - https://github.com/Affirm/incubator-superset/tree/scheduled |
@mahendra Thanks a lot! would you please tell me your email for some questions? I cloned your branch and deploy on my machine, with my Python environment, I changed some function in urllib for Python2.7 and make "ENABLE_SCHEDULED_EMAIL_REPORTS" True, make the "EMAIL_REPORTS_WEBDRIVER" chrome, the "SMTP_HOST" is 'smtp.gmail.com' and port is 587, after set the crontab '*/3 * * * *' but I cannot get report from Superset, did I miss some details? Thanks! |
@YHGui apologies for the delay here. Two things to do:
|
@mistercrunch just checking in on this PR. Should I ping any other reviewers? |
@mahendra thanks! I did it just like you said above several days ago, install selenium and webdriver, run celery worker and celery beat, after running it, I got time limit exceeded and I change the time limit in config.py, but it doesn't work, I'd like to know some settings, such as EMAIL_REPORTS_USER, does it mean the username who is the owner of the dashboard or admin users? |
@magicansk I pull https://github.com/Affirm/incubator-superset set it up, but i still can not see the schedule/email item on the mange menu. how to get it work? |
You need to enable it in configuration. The process is defined in the
documentation. Please do let me know if it doesn't work.
…On Thu, Mar 14, 2019 at 8:39 PM georgexiong ***@***.***> wrote:
@magicansk <https://github.com/magicansk> I pull
https://github.com/Affirm/incubator-superset set it up, but i still can
not see the schedule/email item on the mange menu. how to get it work?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5294 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAHFFPfxxMPmPU2yfr1fHAYDXL2io0asks5vWxXrgaJpZM4U42zO>
.
--
Mahendra
http://twitter.com/mahendra
gpg key: E31C89FD
<https://pgp.mit.edu/pks/lookup?op=get&search=0x7BEC08A5E31C89FD>
|
@mahendra I have enabled email reports. I keep getting this error when celery tries to generate an email:
I have firefox-60.6.1 installed and I am on the release--0.31 branch. |
@YHGui Were you able to solve the problem |
@mahendra How can I merge these changes in my existing 0.29 version? |
@mahendra I tried the email report feature, i am not seeing any errors in the geckodriver.log or nohup for celery worker or superset logs. But i am not getting any emails sent. I checked |
i could make it work by changing the firefox options, just added proxy to it. |
Hi Mahendra, I want to implement schedule reports but the above link got expired. Can you please help me on this? Thanks |
I want to implement schedule reports Can you please help me on this? Thanks |
After performing the configuration tried to run the below command and getting the error: celery worker --app=superset.tasks.celery_app:app --pool=prefork -Ofair -c 4 Error Details****************** The above exception was the direct cause of the following exception: Traceback (most recent call last): Thanks |
@mahendra I have configured everything without any errors but still i am not able to see the email option on front end. Can you please help me with it? |
Set up a local SMTP debugging server and paste the logs here |
Which version are you using?
Have you enabled the option in config.py, which is basically for showing
the menus in superset UI?
…On Thu, 24 Oct, 2019, 6:51 PM prasadbhosale, ***@***.***> wrote:
@mahendra <https://github.com/mahendra> I have configured everything
without any errors but still i am not able to see the email option on front
end. Can you please help me with it?
I am also facing this problem.
@mahendra <https://github.com/mahendra> - Pl help us.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#5294>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEZS2ZKATO7JXDL4YF4GHQDQQGOM5ANCNFSM4FHDNTHA>
.
|
Yes, i have the same problem. Check connection but no opened connections for SMTP . Superset did not fire any email trigger |
We are using superset with schedule report emailing, it works great for us. In the below article, we have published steps to make deployment easy for superset with schedule report emailing. |
Just tried this feature in Superset. Though I got reports delivered through email as a png image the image didnt have actual report data. It just shows screenshot of report running icon. I guess it got delivered before actual report got rendered. Is the report rendering and email sending process asynchronous? Looks like its not waiting for report to get rendered before sending it through email |
I have the same problem. And I have another question, whythe WEBDRIVER_BASEURL port 8080, not 8088? |
i am getting the same error. Could you (or other smart guy) how to fix this issue? |
Can you share a stey-by-step guide, I didin't get any emails when setting by the document(https://superset.incubator.apache.org/installation.html#email-reports). Thanks a lot |
Step1: cd superset/bin |
Can you able to fix this issue? I am using the following version |
Motivation
Scheduled email reports (of dashboards and slices) has been a long standing ask from the superset community. The feature also provides superset a competitive advantage over tools like Looker.
Proposed change
bcc
- for audit purposes.How does this work.
celerybeat
to schedule tasks for delivering reports every hour.celery
task uses selenium (firefox/chrome) webdriver to crawl the actual dashboard (including visualizations) and mail it out to the recipients.New dependencies
New or Changed Public Interfaces
The main menu adds two new entries. This will be organized better in a future PR for categorizing dashboards and slices.
Migration plan
Rejected alternatives
Future work
Screenshots
Admin
Menus
Dashboard schedules
Slice schedules
Delivery
Dashboard via email
Slice via email (inline)
Slice data email (inline)
Slice data as CSV attachment
FAQ's
Why did we not use phantomjs?
(I can make the change if anyone needs/prefers
phantomjs
).phantomjs
had the following issuesphantomjs
, so I stopped working with it.