-
Notifications
You must be signed in to change notification settings - Fork 188
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
Include message with emails #849
Changes from 4 commits
525663b
095f312
a348a4a
6ab9765
2a29eae
b88a2e8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,11 +30,17 @@ | |
|
||
import io.dropwizard.lifecycle.Managed; | ||
|
||
|
||
|
||
@Singleton | ||
public class SingularitySmtpSender implements Managed { | ||
|
||
private static final Logger LOG = LoggerFactory.getLogger(SingularitySmtpSender.class); | ||
|
||
/// Set this to true to log emails being sent. This allows testing locally without setting up an SMTP server | ||
/// To find the email html, grep the logs: "grep TheMail -A 10" (change 10 depending on how many lines you need) | ||
private static final Boolean LOG_EMAILS_FOR_DEBUG = false; | ||
|
||
private final Optional<SMTPConfiguration> maybeSmtpConfiguration; | ||
private final Optional<ThreadPoolExecutor> mailSenderExecutorService; | ||
private final SingularityExceptionNotifier exceptionNotifier; | ||
|
@@ -62,6 +68,10 @@ public void stop() { | |
} | ||
|
||
public void queueMail(final List<String> toList, final List<String> ccList, final String subject, final String body) { | ||
if (LOG_EMAILS_FOR_DEBUG) { | ||
LOG.trace("TheMail: " + body); | ||
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. We can do without the Also for logging, you can so something more like this rather that using "string" + "string" LOG.trace("Sending email: {}, to: {}, cc: {}", body, toList, ccList) 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. For readability I've found it to be easier to put the recipients first and the body last so you're not scrolling through a wall of HTML looking for the recipient at the end |
||
} | ||
|
||
if (toList.isEmpty()) { | ||
LOG.warn("Couldn't queue email {} because no to address is present", subject); | ||
return; | ||
|
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.
super nit-picky, but can we get rid of these extra new lines here and down by the LOG.trace