-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
feat: send report data to Slack #15806
Conversation
Codecov Report
@@ Coverage Diff @@
## master #15806 +/- ##
==========================================
- Coverage 76.83% 76.78% -0.06%
==========================================
Files 988 988
Lines 52195 52321 +126
Branches 7095 7124 +29
==========================================
+ Hits 40106 40173 +67
- Misses 11864 11923 +59
Partials 225 225
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
46aae91
to
8660d28
Compare
@@ -111,6 +111,7 @@ def get_git_sha(): | |||
"sqlalchemy>=1.3.16, <1.4, !=1.3.21", | |||
"sqlalchemy-utils>=0.36.6,<0.37", | |||
"sqlparse==0.3.0", # PINNED! see https://github.com/andialbrecht/sqlparse/issues/562 | |||
"tabulate==0.8.9", |
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.
You will have to update the requirements. Additionally strict pinning of packages in setup.py
is undesirable as it may overly constrain the frozen solution space. Please use limits if strictly necessary.
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.
@john-bodley I did run pip-compile-multi --no-upgrade
, you can see that requirements/base.txt
was updated to include the dependency (it has no additional dependencies of its own).
Also, my understanding is that it's better to pin the version for dependencies with version 0.x
, since there's no guarantee that a minor upgrade will maintain backwards compatibility (https://semver.org/#spec-item-4).
8660d28
to
8727030
Compare
8727030
to
a8b8a8a
Compare
a8b8a8a
to
afa496b
Compare
afa496b
to
11c329c
Compare
11c329c
to
45c977d
Compare
346e6a4
to
382e9ee
Compare
* feat: send data embedded in report email * Change post-processing to use new endpoint * Show TEXT option only to text-based vizs * Fix test * feat: send data embedded in report email * feat: send report data to Slack * Add unit test * trigger tests
* feat: send data embedded in report email * Change post-processing to use new endpoint * Show TEXT option only to text-based vizs * Fix test * feat: send data embedded in report email * feat: send report data to Slack * Add unit test * trigger tests
* feat: send data embedded in report email * Change post-processing to use new endpoint * Show TEXT option only to text-based vizs * Fix test * feat: send data embedded in report email * feat: send report data to Slack * Add unit test * trigger tests
* feat: send data embedded in report email * Change post-processing to use new endpoint * Show TEXT option only to text-based vizs * Fix test * feat: send data embedded in report email * feat: send report data to Slack * Add unit test * trigger tests
SUMMARY
Send embedded report data (see #15805) to Slack when user selects "TEXT" as the format.
When the user selects "TEXT" format and Slack delivery for a chart on a report, we send a truncated table to the channel. Slack limits the pre-formatted text to approximately 25 rows, so the table is truncated at 19 rows.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION