Skip to content

Commit

Permalink
fix: GetTrialRemainingLogRetentionDays should take global log retenti…
Browse files Browse the repository at this point in the history
…on days into account
  • Loading branch information
salonig23 committed Sep 10, 2024
1 parent 8fb9f6b commit d156c93
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 1 deletion.
7 changes: 7 additions & 0 deletions master/internal/api_trials.go
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,13 @@ JOIN run_id_task_id as r ON t.task_id = r.task_id
WHERE r.run_id = ?
`
resp = &apiv1.GetTrialRemainingLogRetentionDaysResponse{}
// set trial retention days to global retention days if
// trial retention days is unset and global retention days is set
if t.LogRetentionDays == nil && a.m.config.RetentionPolicy.LogRetentionDays != nil {
t.LogRetentionDays = a.m.config.RetentionPolicy.LogRetentionDays
}

// if neither trial or global retention days is set, default is forever (-1)
if t.LogRetentionDays == nil || *t.LogRetentionDays == -1 {
days := int32(-1)
resp.RemainingDays = &days
Expand Down
43 changes: 42 additions & 1 deletion master/internal/api_trials_intg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1476,7 +1476,7 @@ func completeExp(ctx context.Context, expID int32) error {
return nil
}

func TestGetTrialRemainingLogRetentionDays(t *testing.T) {
func TestGetTrialRemainingLogRetentionDaysNullMasterConfig(t *testing.T) {
api, _, ctx := setupAPITest(t, nil)

tests := []struct {
Expand Down Expand Up @@ -1515,6 +1515,47 @@ retention_policy:
}
}

func TestGetTrialRemainingLogRetentionDaysNonNullMasterConfig(t *testing.T) {
api, _, ctx := setupAPITest(t, nil)
// set Log retention days in master config
retentionDays := int16(100)
api.m.config.RetentionPolicy.LogRetentionDays = &retentionDays
tests := []struct {
name string
logRetentionDays string
expRemainingDays []int32
}{
{"test-null-days", "", []int32{84, 89, 94, 99}},
{"test-forever", `
retention_policy:
log_retention_days: -1
`, []int32{-1, -1, -1, -1}},
{"test-10days", `
retention_policy:
log_retention_days: 10
`, []int32{0, 0, 4, 9}},
}

for _, tt := range tests {
log.Printf("Starting %v", tt.name)
testEndTimes := []int{15, 10, 5, 0}
exp, trialIDs, _ := CreateTestRetentionExperiment(ctx, t, api, tt.logRetentionDays, len(testEndTimes))

for i, v := range testEndTimes {
err := completeTrialsandTasks(ctx, trialIDs[i], v)
require.NoError(t, err)

d, err := api.GetTrialRemainingLogRetentionDays(ctx, &apiv1.GetTrialRemainingLogRetentionDaysRequest{
Id: int32(trialIDs[i]),
})
require.NoError(t, err)
require.Equal(t, tt.expRemainingDays[i], *d.RemainingDays)
}
err := completeExp(ctx, exp.Id)
require.NoError(t, err)
}
}

func TestRunLocalID(t *testing.T) {
api, curUser, ctx := setupAPITest(t, nil)
_, projectID := createProjectAndWorkspace(ctx, t, api)
Expand Down

0 comments on commit d156c93

Please sign in to comment.