-
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
Merged
Merged
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
74e8b29
[scheduled reports] Add support for scheduled reports
26e3946
[scheduled reports] - Debug mode for scheduled emails
3673dd7
[scheduled reports] - Ability to send test mails
ca54352
[scheduled reports] - Test email functionality - minor improvements
5323236
[scheduled reports] - Rebase with master. Minor fixes
528776c
[scheduled reports] - Add warning messages
a4261af
[scheduled reports] - flake8
ab0124c
[scheduled reports] - fix rebase
1d0e7ea
[scheduled reports] - fix rebase
9f55872
[scheduled reports] - fix flake8
d30bdae
[scheduled reports] Rebase in prep for merge
de99f3e
[scheduled reports] - fix flake8
2be4eec
[scheduled reports] - address review comments
8886068
[scheduled reports] - rebase with master
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,7 @@ | |
import os | ||
import sys | ||
|
||
from celery.schedules import crontab | ||
from dateutil import tz | ||
from flask_appbuilder.security.manager import AUTH_DB | ||
|
||
|
@@ -296,19 +297,43 @@ | |
# Default celery config is to use SQLA as a broker, in a production setting | ||
# you'll want to use a proper broker as specified here: | ||
# http://docs.celeryproject.org/en/latest/getting-started/brokers/index.html | ||
""" | ||
# Example: | ||
|
||
|
||
class CeleryConfig(object): | ||
BROKER_URL = 'sqla+sqlite:///celerydb.sqlite' | ||
CELERY_IMPORTS = ('superset.sql_lab', ) | ||
CELERY_RESULT_BACKEND = 'db+sqlite:///celery_results.sqlite' | ||
CELERY_ANNOTATIONS = {'tasks.add': {'rate_limit': '10/s'}} | ||
CELERYD_LOG_LEVEL = 'DEBUG' | ||
CELERYD_PREFETCH_MULTIPLIER = 1 | ||
CELERY_ACKS_LATE = True | ||
BROKER_URL = 'sqla+sqlite:///celerydb.sqlite' | ||
CELERY_IMPORTS = ( | ||
'superset.sql_lab', | ||
'superset.tasks', | ||
) | ||
CELERY_RESULT_BACKEND = 'db+sqlite:///celery_results.sqlite' | ||
CELERYD_LOG_LEVEL = 'DEBUG' | ||
CELERYD_PREFETCH_MULTIPLIER = 1 | ||
CELERY_ACKS_LATE = True | ||
CELERY_ANNOTATIONS = { | ||
'sql_lab.get_sql_results': { | ||
'rate_limit': '100/s', | ||
}, | ||
'email_reports.send': { | ||
'rate_limit': '1/s', | ||
'time_limit': 120, | ||
'soft_time_limit': 150, | ||
'ignore_result': True, | ||
}, | ||
} | ||
CELERYBEAT_SCHEDULE = { | ||
'email_reports.schedule_hourly': { | ||
'task': 'email_reports.schedule_hourly', | ||
'schedule': crontab(minute=1, hour='*'), | ||
}, | ||
} | ||
|
||
|
||
CELERY_CONFIG = CeleryConfig | ||
|
||
""" | ||
# Set celery config to None to disable all the above configuration | ||
CELERY_CONFIG = None | ||
""" | ||
|
||
# static http headers to be served by your Superset server. | ||
# This header prevents iFrames from other domains and | ||
|
@@ -450,6 +475,54 @@ class CeleryConfig(object): | |
# using flask-compress | ||
ENABLE_FLASK_COMPRESS = True | ||
|
||
# Enable / disable scheduled email reports | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mistercrunch config to hide the feature. |
||
ENABLE_SCHEDULED_EMAIL_REPORTS = False | ||
|
||
# If enabled, certail features are run in debug mode | ||
# Current list: | ||
# * Emails are sent using dry-run mode (logging only) | ||
SCHEDULED_EMAIL_DEBUG_MODE = False | ||
|
||
# Email reports - minimum time resolution (in minutes) for the crontab | ||
EMAIL_REPORTS_CRON_RESOLUTION = 15 | ||
|
||
# Email report configuration | ||
# From address in emails | ||
EMAIL_REPORT_FROM_ADDRESS = 'reports@superset.org' | ||
|
||
# Send bcc of all reports to this address. Set to None to disable. | ||
# This is useful for maintaining an audit trail of all email deliveries. | ||
EMAIL_REPORT_BCC_ADDRESS = None | ||
|
||
# User credentials to use for generating reports | ||
# This user should have permissions to browse all the dashboards and | ||
# slices. | ||
# TODO: In the future, login as the owner of the item to generate reports | ||
EMAIL_REPORTS_USER = 'admin' | ||
EMAIL_REPORTS_SUBJECT_PREFIX = '[Report] ' | ||
|
||
# The webdriver to use for generating reports. Use one of the following | ||
# firefox | ||
# Requires: geckodriver and firefox installations | ||
# Limitations: can be buggy at times | ||
# chrome: | ||
# Requires: headless chrome | ||
# Limitations: unable to generate screenshots of elements | ||
EMAIL_REPORTS_WEBDRIVER = 'firefox' | ||
|
||
# Window size - this will impact the rendering of the data | ||
WEBDRIVER_WINDOW = { | ||
'dashboard': (1600, 2000), | ||
'slice': (3000, 1200), | ||
} | ||
|
||
# Any config options to be passed as-is to the webdriver | ||
WEBDRIVER_CONFIGURATION = {} | ||
|
||
# The base URL to query for accessing the user interface | ||
WEBDRIVER_BASEURL = 'http://0.0.0.0:8080/' | ||
|
||
|
||
try: | ||
if CONFIG_PATH_ENV_VAR in os.environ: | ||
# Explicitly import config module that is not in pythonpath; useful | ||
|
69 changes: 69 additions & 0 deletions
69
superset/migrations/versions/6c7537a6004a_models_for_email_reports.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
"""models for email reports | ||
|
||
Revision ID: 6c7537a6004a | ||
Revises: e502db2af7be | ||
Create Date: 2018-05-15 20:28:51.977572 | ||
|
||
""" | ||
|
||
# revision identifiers, used by Alembic. | ||
revision = '6c7537a6004a' | ||
down_revision = 'a61b40f9f57f' | ||
|
||
from alembic import op | ||
import sqlalchemy as sa | ||
|
||
|
||
def upgrade(): | ||
# ### commands auto generated by Alembic - please adjust! ### | ||
op.create_table('dashboard_email_schedules', | ||
sa.Column('created_on', sa.DateTime(), nullable=True), | ||
sa.Column('changed_on', sa.DateTime(), nullable=True), | ||
sa.Column('id', sa.Integer(), nullable=False), | ||
sa.Column('active', sa.Boolean(), nullable=True), | ||
sa.Column('crontab', sa.String(length=50), nullable=True), | ||
sa.Column('recipients', sa.Text(), nullable=True), | ||
sa.Column('deliver_as_group', sa.Boolean(), nullable=True), | ||
sa.Column('delivery_type', sa.Enum('attachment', 'inline', name='emaildeliverytype'), nullable=True), | ||
sa.Column('dashboard_id', sa.Integer(), nullable=True), | ||
sa.Column('created_by_fk', sa.Integer(), nullable=True), | ||
sa.Column('changed_by_fk', sa.Integer(), nullable=True), | ||
sa.Column('user_id', sa.Integer(), nullable=True), | ||
sa.ForeignKeyConstraint(['changed_by_fk'], ['ab_user.id'], ), | ||
sa.ForeignKeyConstraint(['created_by_fk'], ['ab_user.id'], ), | ||
sa.ForeignKeyConstraint(['dashboard_id'], ['dashboards.id'], ), | ||
sa.ForeignKeyConstraint(['user_id'], ['ab_user.id'], ), | ||
sa.PrimaryKeyConstraint('id') | ||
) | ||
op.create_index(op.f('ix_dashboard_email_schedules_active'), 'dashboard_email_schedules', ['active'], unique=False) | ||
op.create_table('slice_email_schedules', | ||
sa.Column('created_on', sa.DateTime(), nullable=True), | ||
sa.Column('changed_on', sa.DateTime(), nullable=True), | ||
sa.Column('id', sa.Integer(), nullable=False), | ||
sa.Column('active', sa.Boolean(), nullable=True), | ||
sa.Column('crontab', sa.String(length=50), nullable=True), | ||
sa.Column('recipients', sa.Text(), nullable=True), | ||
sa.Column('deliver_as_group', sa.Boolean(), nullable=True), | ||
sa.Column('delivery_type', sa.Enum('attachment', 'inline', name='emaildeliverytype'), nullable=True), | ||
sa.Column('slice_id', sa.Integer(), nullable=True), | ||
sa.Column('email_format', sa.Enum('visualization', 'data', name='sliceemailreportformat'), nullable=True), | ||
sa.Column('created_by_fk', sa.Integer(), nullable=True), | ||
sa.Column('changed_by_fk', sa.Integer(), nullable=True), | ||
sa.Column('user_id', sa.Integer(), nullable=True), | ||
sa.ForeignKeyConstraint(['changed_by_fk'], ['ab_user.id'], ), | ||
sa.ForeignKeyConstraint(['created_by_fk'], ['ab_user.id'], ), | ||
sa.ForeignKeyConstraint(['slice_id'], ['slices.id'], ), | ||
sa.ForeignKeyConstraint(['user_id'], ['ab_user.id'], ), | ||
sa.PrimaryKeyConstraint('id') | ||
) | ||
op.create_index(op.f('ix_slice_email_schedules_active'), 'slice_email_schedules', ['active'], unique=False) | ||
# ### end Alembic commands ### | ||
|
||
|
||
def downgrade(): | ||
# ### commands auto generated by Alembic - please adjust! ### | ||
op.drop_index(op.f('ix_slice_email_schedules_active'), table_name='slice_email_schedules') | ||
op.drop_table('slice_email_schedules') | ||
op.drop_index(op.f('ix_dashboard_email_schedules_active'), table_name='dashboard_email_schedules') | ||
op.drop_table('dashboard_email_schedules') | ||
# ### end Alembic commands ### |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
from . import core # noqa | ||
from . import sql_lab # noqa | ||
from . import user_attributes # noqa | ||
from . import schedules # noqa |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Nit: is there a reason why these are pegged to these specific versions, and not unpegged,
<=
or>=
? The version ofcroniter
is current, butselenium
has had two minor releases since, with the latest yesterday. If there are no known compatibility issues with either prior or future versions, perhaps these should be left unpegged insetup.py
?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.
@villebro thank you for the comment. It was mostly for a safe bet, to avoid breakages. Specially with selenium. I wanted to peg it to a version which I knew worked. Selenium versions and some webdrivers end up having compatibility issues. I am OK with leaving it unpegged, but I don't think our testing is good enough to catch future breakages.
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.
@mahendra IMO fixing deprecations and possible future breakages tends to be fairly straight forward, and shouldn't be a big issue. So from a maintainability perspective leaving them unpegged would probably be better, as it would allow for features/fixes to flow in automatically.
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.
@villebro Noted. I just updated the code and removed the pinning. Pushed it to github.
To clarify, my worry was not about the ability to fix breakages. Rather it was to avoid breaking deploys. Specifically when someone installs/upgrades to superset==0.27.0 and their setup script accidentally upgrades selenium to an incompatible version. This could leave them with a broken deploy. We could fix it once we get bug 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.
@mahendra I understand, perhaps @betodealmeida or one of the official maintainers can weigh in? As there doesn't seem to be any known incompatibility issues with
croniter>0.3.24
andselenium>3.12.0
, and this is a new feature, I think any users testing it will be aware that they should expect a bug or two as this is rolled out. If incompatibility issues do arise, I feel they are best taken care of by making changes to the Superset code to accomodate API changes etc. Only in the case of a serious divergence in features or known bugs should we pin to an old version inmaster
. Perhaps even fixing the dependency versions insetup.py
of0.27
might be an option.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.
@villebro agree with you. I have unpinned the versions. We can fix it in the future if required.
@betodealmeida thoughts?