Skip to content

Commit

Permalink
fix: issue when calculating window interval (#202)
Browse files Browse the repository at this point in the history
* fix: properly handle a case where custom window v3 is used

* Revert "fix: properly handle a case where custom window v3 is used"

This reverts commit 264ebfd.

* fix: properly handle interval calculation for window v3
  • Loading branch information
irainia authored Mar 19, 2024
1 parent 38d9c01 commit 809eefd
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 9 deletions.
31 changes: 22 additions & 9 deletions core/scheduler/service/job_run_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/goto/optimus/core/tenant"
"github.com/goto/optimus/internal/errors"
"github.com/goto/optimus/internal/lib/cron"
"github.com/goto/optimus/internal/lib/duration"
"github.com/goto/optimus/internal/lib/interval"
"github.com/goto/optimus/internal/lib/window"
"github.com/goto/optimus/internal/models"
Expand Down Expand Up @@ -197,7 +198,9 @@ func (s *JobRunService) GetInterval(ctx context.Context, projectName tenant.Proj

// TODO: this method is only for backward compatibility, it will be deprecated soon
func (s *JobRunService) getInterval(project *tenant.Project, job *scheduler.JobWithDetails, referenceTime time.Time) (interval.Interval, error) {
if job.Job.WindowConfig.Type() == window.Incremental {
windowConfig := job.Job.WindowConfig

if windowConfig.Type() == window.Incremental {
w, err := window.FromSchedule(job.Schedule.Interval)
if err != nil {
s.l.Error("error getting window with type incremental: %v", err)
Expand All @@ -207,21 +210,31 @@ func (s *JobRunService) getInterval(project *tenant.Project, job *scheduler.JobW
return w.GetInterval(referenceTime)
}

if job.Job.WindowConfig.Type() == window.Preset {
preset, err := project.GetPreset(job.Job.WindowConfig.Preset)
if err != nil {
s.l.Error("error getting preset [%s] for project [%s]: %v", job.Job.WindowConfig.Preset, project.Name(), err)
return interval.Interval{}, err
if windowConfig.Type() == window.Preset || windowConfig.GetVersion() == window.NewWindowVersion {
var config window.SimpleConfig
if windowConfig.Type() == window.Preset {
preset, err := project.GetPreset(windowConfig.Preset)
if err != nil {
s.l.Error("error getting preset [%s] for project [%s]: %v", windowConfig.Preset, project.Name(), err)
return interval.Interval{}, err
}

config = preset.Config()
} else {
config = windowConfig.GetSimpleConfig()
}
cw, err := window.FromCustomConfig(preset.Config())

config.Delay = ""
config.TruncateTo = string(duration.None)

cw, err := window.FromCustomConfig(config)
if err != nil {
return interval.Interval{}, err
}
return cw.GetInterval(referenceTime)
}

baseWindow := job.Job.WindowConfig.Window
w, err := models.NewWindow(baseWindow.GetVersion(), "", "0", baseWindow.GetSize())
w, err := models.NewWindow(windowConfig.GetVersion(), "", "0", windowConfig.GetSize())
if err != nil {
s.l.Error("error initializing window: %v", err)
return interval.Interval{}, err
Expand Down
35 changes: 35 additions & 0 deletions core/scheduler/service/job_run_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1398,6 +1398,41 @@ func TestJobRunService(t *testing.T) {
assert.ErrorContains(t, actualError, "unexpected error")
})

t.Run("returns interval and nil if custom window v3 is used", func(t *testing.T) {
projectGetter := new(mockProjectGetter)
defer projectGetter.AssertExpectations(t)

jobRepo := new(JobRepository)
defer jobRepo.AssertExpectations(t)

projectGetter.On("Get", ctx, projName).Return(project, nil)

windowConfig, err := window.NewConfig(preset.Config().Size, preset.Config().Delay, preset.Config().Location, preset.Config().TruncateTo)
assert.NotNil(t, windowConfig)
assert.NoError(t, err)

job := &scheduler.JobWithDetails{
Name: jobName,
Schedule: &scheduler.Schedule{
Interval: "0 * * * *",
},
Job: &scheduler.Job{
WindowConfig: windowConfig,
},
JobMetadata: &scheduler.JobMetadata{
Version: window.NewWindowVersion,
},
}

jobRepo.On("GetJobDetails", ctx, projName, jobName).Return(job, nil)
service := service.NewJobRunService(logger, jobRepo, nil, nil, nil, nil, nil, nil, nil, projectGetter)

actualInterval, actualError := service.GetInterval(ctx, projName, jobName, referenceTime)

assert.NotZero(t, actualInterval)
assert.NoError(t, actualError)
})

t.Run("returns interval and nil if no error is encountered", func(t *testing.T) {
projectGetter := new(mockProjectGetter)
defer projectGetter.AssertExpectations(t)
Expand Down

0 comments on commit 809eefd

Please sign in to comment.