From 4fef8a5fa729f640c0779dce3584f018e8b2006c Mon Sep 17 00:00:00 2001 From: Ian McEwen Date: Wed, 18 Sep 2024 10:11:10 -0700 Subject: [PATCH 1/6] Use the nicer duration string for the notification body within the DE as well --- main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main.go b/main.go index 3165b1c..514cde1 100644 --- a/main.go +++ b/main.go @@ -184,7 +184,7 @@ func SendPeriodicNotification(ctx context.Context, j *Job) error { j.Name, j.ID, starttime, - dur, + durString, ) return sendNotif(ctx, j, j.Status, subject, msg) From 07c1b5c81c97bddf90aac931061f3b7a0d635c53 Mon Sep 17 00:00:00 2001 From: Ian McEwen Date: Wed, 18 Sep 2024 10:16:24 -0700 Subject: [PATCH 2/6] Split out notification stuff to allow passing a custom template if desired --- main.go | 2 +- notifications.go | 9 +++++++-- notifications_test.go | 6 +++--- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/main.go b/main.go index 514cde1..9ce5d85 100644 --- a/main.go +++ b/main.go @@ -77,7 +77,7 @@ func sendNotif(ctx context.Context, j *Job, status, subject, msg string) error { p.Email = user.Email p.User = u - notif := NewNotification(u, subject, msg, p) + notif := NewStatusChangeNotification(u, subject, msg, p) resp, err := notif.Send(ctx) if err != nil { diff --git a/notifications.go b/notifications.go index 4c895d8..c183b6f 100644 --- a/notifications.go +++ b/notifications.go @@ -84,7 +84,7 @@ func NewPayload() *Payload { } // NewNotification returns a newly initialized *Notification. -func NewNotification(user, subject, msg string, payload *Payload) *Notification { +func NewNotification(user, subject, msg, emailTemplate string, payload *Payload) *Notification { return &Notification{ URI: NotifsURI, Type: "analysis", @@ -92,11 +92,16 @@ func NewNotification(user, subject, msg string, payload *Payload) *Notification Subject: subject, Message: msg, Email: true, - EmailTemplate: "analysis_status_change", + EmailTemplate: emailTemplate, Payload: payload, } } +// NewStatusChangeNotification returns a newly initialized *Notification using the analysis_status_change email template +func NewStatusChangeNotification(user, subject, msg string, payload *Payload) *Notification { + return NewNotification(user, subject, msg, "analysis_status_change", payload) +} + // Send POSTs the notification to the URI. func (n *Notification) Send(ctx context.Context) (*http.Response, error) { msg, err := json.Marshal(n) diff --git a/notifications_test.go b/notifications_test.go index 1228eee..7761242 100644 --- a/notifications_test.go +++ b/notifications_test.go @@ -18,12 +18,12 @@ func TestNotifsInit(t *testing.T) { } } -func TestNewNotification(t *testing.T) { +func TestNewStatusChangeNotification(t *testing.T) { expectedURI := "foo" expectedUser := "user" expectedSubject := "subject" NotifsInit(expectedURI) - n := NewNotification(expectedUser, expectedSubject, "", nil) + n := NewStatusChangeNotification(expectedUser, expectedSubject, "", nil) if n.URI != expectedURI { t.Errorf("URI was %s, not %s", n.URI, expectedURI) } @@ -38,7 +38,7 @@ func TestNewNotification(t *testing.T) { func TestSend(t *testing.T) { expectedUser := "test-user" expectedSubject := "test-subject" - n := NewNotification(expectedUser, expectedSubject, "", nil) + n := NewStatusChangeNotification(expectedUser, expectedSubject, "", nil) srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { b, err := io.ReadAll(r.Body) From 383c11af6b31a1f3d0ceaab4d6caf1ffc99f34a9 Mon Sep 17 00:00:00 2001 From: Ian McEwen Date: Wed, 18 Sep 2024 10:30:12 -0700 Subject: [PATCH 3/6] Split job duration string calculation into its own function --- analyses.go | 15 +++++++++++++++ main.go | 11 ++--------- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/analyses.go b/analyses.go index 049bbad..8249f0a 100644 --- a/analyses.go +++ b/analyses.go @@ -41,6 +41,21 @@ type Job struct { ExternalID string `json:"external_id"` } +// getJobDuration takes a job and returns a duration string and the start time of the job +func getJobDuration(j *Job) (string, time.Time, error) { + starttime, err := time.ParseInLocation(TimestampFromDBFormat, j.StartDate, time.Local) + if err != nil { + return "", starttime, errors.Wrapf(err, "failed to parse start date %s", j.StartDate) + } + + // just print H(HH):MM format + dur := time.Since(starttime).Round(time.Minute) + h := dur / time.Hour + dur -= h * time.Hour + m := dur / time.Minute + return fmt.Sprintf("%d:%02d", h, m), starttime, nil +} + const jobsToKillQuery = ` select jobs.id, jobs.app_id, diff --git a/main.go b/main.go index 9ce5d85..d25a47c 100644 --- a/main.go +++ b/main.go @@ -165,18 +165,11 @@ func SendWarningNotification(ctx context.Context, j *Job) error { } func SendPeriodicNotification(ctx context.Context, j *Job) error { - starttime, err := time.ParseInLocation(TimestampFromDBFormat, j.StartDate, time.Local) + durString, starttime, err := getJobDuration(j) if err != nil { - return errors.Wrapf(err, "failed to parse start date %s", j.StartDate) + return err } - // just print H(HH):MM format - dur := time.Since(starttime).Round(time.Minute) - h := dur / time.Hour - dur -= h * time.Hour - m := dur / time.Minute - durString := fmt.Sprintf("%d:%02d", h, m) - subject := fmt.Sprintf(PeriodicSubjectFormat, j.Name, starttime, durString) msg := fmt.Sprintf( From 7b748503d17e76df6b2d1d9dca3600281ff3bd13 Mon Sep 17 00:00:00 2001 From: Ian McEwen Date: Wed, 18 Sep 2024 10:37:13 -0700 Subject: [PATCH 4/6] Pass a run duration in the notification payload to be used by templates --- main.go | 6 ++++++ notifications.go | 1 + 2 files changed, 7 insertions(+) diff --git a/main.go b/main.go index d25a47c..eaa63b0 100644 --- a/main.go +++ b/main.go @@ -68,12 +68,18 @@ func sendNotif(ctx context.Context, j *Job, status, subject, msg string) error { } sdmillis := sd.UnixNano() / 1000000 + durString, _, err := getJobDuration(j) + if err != nil { + return errors.Wrapf(err, "failed to parse job duration from %s", j.StartDate) + } + p := NewPayload() p.AnalysisName = j.Name p.AnalysisDescription = j.Description p.AnalysisStatus = status p.AnalysisStartDate = strconv.FormatInt(sdmillis, 10) p.AnalysisResultsFolder = j.ResultFolder + p.RunDuration = durString p.Email = user.Email p.User = u diff --git a/notifications.go b/notifications.go index c183b6f..359f16b 100644 --- a/notifications.go +++ b/notifications.go @@ -71,6 +71,7 @@ type Payload struct { AnalysisStatus string `json:"analysisstatus"` AnalysisStartDate string `json:"analysisstartdate"` AnalysisResultsFolder string `json:"analysisresultsfolder"` + RunDuration string `json:"runduration"` Email string `json:"email_address"` Action string `json:"action"` User string `json:"user"` From b96699f382ef91b6d39b3059cd9aee41a8742108 Mon Sep 17 00:00:00 2001 From: Ian McEwen Date: Wed, 18 Sep 2024 10:40:50 -0700 Subject: [PATCH 5/6] Use a new template name for periodic warnings --- main.go | 10 +++++----- notifications.go | 5 ----- notifications_test.go | 10 +++++++--- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/main.go b/main.go index eaa63b0..c17fbd9 100644 --- a/main.go +++ b/main.go @@ -45,7 +45,7 @@ iplant_groups: const warningSentKey = "warningsent" const oneDayWarningKey = "onedaywarning" -func sendNotif(ctx context.Context, j *Job, status, subject, msg string) error { +func sendNotif(ctx context.Context, j *Job, status, subject, msg, email_template string) error { var err error // Don't send notification if things aren't configured correctly. It's @@ -83,7 +83,7 @@ func sendNotif(ctx context.Context, j *Job, status, subject, msg string) error { p.Email = user.Email p.User = u - notif := NewStatusChangeNotification(u, subject, msg, p) + notif := NewNotification(u, subject, msg, email_template, p) resp, err := notif.Send(ctx) if err != nil { @@ -143,7 +143,7 @@ func SendKillNotification(ctx context.Context, j *Job, killNotifKey string) erro endtime.UTC().Format(time.UnixDate), j.ResultFolder, ) - err = sendNotif(ctx, j, "Canceled", subject, msg) + err = sendNotif(ctx, j, "Canceled", subject, msg, "analysis_status_change") return err } @@ -167,7 +167,7 @@ func SendWarningNotification(ctx context.Context, j *Job) error { j.ResultFolder, ) - return sendNotif(ctx, j, j.Status, subject, msg) + return sendNotif(ctx, j, j.Status, subject, msg, "analysis_status_change") } func SendPeriodicNotification(ctx context.Context, j *Job) error { @@ -186,7 +186,7 @@ func SendPeriodicNotification(ctx context.Context, j *Job) error { durString, ) - return sendNotif(ctx, j, j.Status, subject, msg) + return sendNotif(ctx, j, j.Status, subject, msg, "analysis_periodic_notification") } func ensureNotifRecord(ctx context.Context, vicedb *VICEDatabaser, job Job) error { diff --git a/notifications.go b/notifications.go index 359f16b..67c620d 100644 --- a/notifications.go +++ b/notifications.go @@ -98,11 +98,6 @@ func NewNotification(user, subject, msg, emailTemplate string, payload *Payload) } } -// NewStatusChangeNotification returns a newly initialized *Notification using the analysis_status_change email template -func NewStatusChangeNotification(user, subject, msg string, payload *Payload) *Notification { - return NewNotification(user, subject, msg, "analysis_status_change", payload) -} - // Send POSTs the notification to the URI. func (n *Notification) Send(ctx context.Context) (*http.Response, error) { msg, err := json.Marshal(n) diff --git a/notifications_test.go b/notifications_test.go index 7761242..1de7cb4 100644 --- a/notifications_test.go +++ b/notifications_test.go @@ -18,12 +18,13 @@ func TestNotifsInit(t *testing.T) { } } -func TestNewStatusChangeNotification(t *testing.T) { +func TestNewNotification(t *testing.T) { expectedURI := "foo" expectedUser := "user" expectedSubject := "subject" + expectedTemplate := "analysis_status_change" NotifsInit(expectedURI) - n := NewStatusChangeNotification(expectedUser, expectedSubject, "", nil) + n := NewNotification(expectedUser, expectedSubject, "", expectedTemplate, nil) if n.URI != expectedURI { t.Errorf("URI was %s, not %s", n.URI, expectedURI) } @@ -33,12 +34,15 @@ func TestNewStatusChangeNotification(t *testing.T) { if n.Subject != expectedSubject { t.Errorf("Subject was %s, not %s", n.Subject, expectedSubject) } + if n.EmailTemplate != expectedTemplate { + t.Errorf("EmailTemplate was %s, not %s", n.EmailTemplate, expectedTemplate) + } } func TestSend(t *testing.T) { expectedUser := "test-user" expectedSubject := "test-subject" - n := NewStatusChangeNotification(expectedUser, expectedSubject, "", nil) + n := NewNotification(expectedUser, expectedSubject, "", "analysis_status_change", nil) srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { b, err := io.ReadAll(r.Body) From ff34ec40ea422aa6b3fa4e3934373d61ae52afce Mon Sep 17 00:00:00 2001 From: Ian McEwen Date: Wed, 18 Sep 2024 11:01:54 -0700 Subject: [PATCH 6/6] Add detail to function comment --- notifications.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/notifications.go b/notifications.go index 67c620d..64fd2ad 100644 --- a/notifications.go +++ b/notifications.go @@ -77,7 +77,7 @@ type Payload struct { User string `json:"user"` } -// NewPayload returns a newly constructed *Payload +// NewPayload returns a newly constructed *Payload with the Action set to "job_status_change" func NewPayload() *Payload { return &Payload{ Action: "job_status_change",