rob05c commented on a change in pull request #3929: Rewrote
/user/reset_password to Go
URL: https://github.com/apache/trafficcontrol/pull/3929#discussion_r327200950
##########
File path: traffic_ops/traffic_ops_golang/api/api.go
##########
@@ -369,6 +371,33 @@ func (inf *APIInfo) Close() {
}
}
+// SendMail is a convenience method used to call SendMail using an APIInfo
structure's configuration.
+func (inf *APIInfo) SendMail(to rfc.EmailAddress, msg []byte) (int, error,
error) {
+ return SendMail(to, msg, inf.Config)
+}
+
+// SendMail sends an email msg to the address identified by to. The msg
parameter should be an
+// RFC822-style email with headers first, a blank line, and then the message
body. The lines of msg
+// should be CRLF terminated. The msg headers should usually include fields
such as "From", "To",
+// "Subject", and "Cc". Sending "Bcc" messages is accomplished by including an
email address in the
+// to parameter but not including it in the msg headers.
+// The cfg parameter is used to set things like the "From" field, as well as
for connection
+// and authentication with an external SMTP server.
+// SendMail returns (in order) an HTTP status code, a user-friendly error, and
an error fit for
+// logging to system error logs. If either the user or system error is
non-nil, the operation failed,
+// and the HTTP status code indicates the type of failure.
+func SendMail(to rfc.EmailAddress, msg []byte, cfg *config.Config) (int,
error, error) {
+ if !cfg.SMTP.Enabled {
+ return http.StatusServiceUnavailable, errors.New("SMTP is not
enabled!"), nil
+ }
+ auth := smtp.PlainAuth("", cfg.SMTP.User, cfg.SMTP.Password,
strings.Split(cfg.SMTP.Address, ":")[0])
+ err := smtp.SendMail(cfg.SMTP.Address, auth,
cfg.ConfigTO.EmailFrom.String(), []string{to.String()}, msg)
+ if err != nil {
+ return http.StatusBadGateway, nil, err
Review comment:
A 502 feels strange here to me, too. The server was acting as a "gateway" in
that it asked another service for something. But TO itself isn't a Proxy. I
think a 502 might be technically correct, but as a client, I wouldn't expect to
ever receive that from TO. Clients think of TO as a service they interact with,
not a Proxy or Gateway.
I wouldn't hold up the merge for it, but a 502 from TO definitely feels
unexpected to me, and seems like it would cause more confusion than clarity to
clients.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services