-
Notifications
You must be signed in to change notification settings - Fork 14k
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
chore: change subject name from no_name to named for PNGs in #19929
Conversation
Codecov Report
@@ Coverage Diff @@
## master #19929 +/- ##
==========================================
- Coverage 66.51% 66.35% -0.16%
==========================================
Files 1715 1715
Lines 65060 65063 +3
Branches 6723 6723
==========================================
- Hits 43272 43170 -102
- Misses 20076 20181 +105
Partials 1712 1712
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
f1d856c
to
518d6c6
Compare
/testenv up FEATURE_ALERT_REPORTS=True |
@AAfghahi Ephemeral environment spinning up at http://54.214.133.2:8080. Credentials are |
superset/utils/core.py
Outdated
image.add_header("Content-ID", "<%s>" % msgid) | ||
image.add_header("Content-Disposition", "inline") | ||
image.add_header("Content-Disposition", 'attachment; filename="%s"' % file_name) |
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.
what's the difference between this line, and what we're doing in line 937?
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.
good question, I am running it now without the file in image to see. The documentation used both so I emulated and then wasn't able to properly test until just recently.
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.
Ok, I tested, and the add_header line seems inconsequential to the process that we want, so I deleted it.
518d6c6
to
a8c2f16
Compare
screenshot looks good to me! |
Ephemeral environment shutdown and build artifacts deleted. |
SUMMARY
Previously images in reports or alerts looked like this:
this PR changes them to have this:
[chart or dashboard name] [date of screenshot].png
After:
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION