diff --git a/client/cmd/playground/window/v1v2/constant.go b/client/cmd/playground/window/v1v2/constant.go deleted file mode 100644 index 41bbf2c35c..0000000000 --- a/client/cmd/playground/window/v1v2/constant.go +++ /dev/null @@ -1,24 +0,0 @@ -package v1v2 - -type cursorPointer string - -const ( - pointToYear cursorPointer = "year" - pointToMonth cursorPointer = "month" - pointToDay cursorPointer = "day" - pointToHour cursorPointer = "hour" - pointToMinute cursorPointer = "minute" - - pointToTruncateTo cursorPointer = "truncate_to" - pointToOffset cursorPointer = "offset" - pointToSize cursorPointer = "size" -) - -type truncateTo string - -const ( - truncateToMonth truncateTo = "M" - truncateToWeek truncateTo = "w" - truncateToDay truncateTo = "d" - truncateToHour truncateTo = "h" -) diff --git a/client/cmd/playground/window/v1v2/doc.go b/client/cmd/playground/window/v1v2/doc.go deleted file mode 100644 index ef884bcb40..0000000000 --- a/client/cmd/playground/window/v1v2/doc.go +++ /dev/null @@ -1,4 +0,0 @@ -// v1v2 package is a temporary playground package for window v1 and v2. -// Along with deprecation of window v1 and v2, this package will also be removed. -// This makes playground to be only for window v3. -package v1v2 diff --git a/client/cmd/playground/window/v1v2/model.go b/client/cmd/playground/window/v1v2/model.go deleted file mode 100644 index 5e6bf9fb7d..0000000000 --- a/client/cmd/playground/window/v1v2/model.go +++ /dev/null @@ -1,374 +0,0 @@ -package v1v2 - -import ( - "bytes" - "fmt" - "reflect" - "strconv" - "strings" - "time" - - "github.com/charmbracelet/bubbles/textinput" - tea "github.com/charmbracelet/bubbletea" - "github.com/olekukonko/tablewriter" - - "github.com/goto/optimus/internal/models" -) - -type model struct { - currentCursor cursorPointer - - truncateTo truncateTo - sizeInput textinput.Model - offsetInput textinput.Model - - scheduledTime time.Time -} - -func NewModel() *model { - return &model{ - currentCursor: pointToTruncateTo, - truncateTo: truncateToDay, - sizeInput: textinput.New(), - offsetInput: textinput.New(), - scheduledTime: time.Now(), - } -} - -func (*model) Init() tea.Cmd { - // this method is to adhere to library contract - return nil -} - -func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { - currMsg := reflect.TypeOf(msg) - if currMsg.String() != "tea.KeyMsg" { - return m, nil - } - - msgStr := fmt.Sprintf("%s", msg) - switch msgStr { - case "ctrl+c", "q": - return m, tea.Quit - case "up", "w": - m.handleUp() - case "down", "s": - m.handleDown() - case "left", "a": - m.handleLeft() - case "right", "d": - m.handleRight() - case "shift+up", "W": - m.handleIncrement() - case "shift+down", "S": - m.handleDecrement() - case "M", "h", "-", - "1", "2", "3", "4", "5", - "6", "7", "8", "9", "0", - "backspace": - m.handleInput(msg) - } - return m, nil -} - -func (m *model) View() string { - var s strings.Builder - s.WriteString("INPUT") - s.WriteString("\n") - s.WriteString(m.generateWindowInputView()) - s.WriteString("\n") - s.WriteString(m.generateWindowInputHintView()) - s.WriteString("\n") - s.WriteString("RESULT") - s.WriteString("\n") - s.WriteString(m.generateWindowResultView()) - s.WriteString("\n") - s.WriteString("DOCUMENTATION:\n") - s.WriteString("- https://goto.github.io/optimus/docs/concepts/intervals-and-windows") - return s.String() -} - -func (m *model) generateWindowResultView() string { - buff := &bytes.Buffer{} - table := tablewriter.NewWriter(buff) - table.SetHeader([]string{"Version", "Start Time", "End Time"}) - table.Append(m.generateWindowTableRowView(1)) - table.Append(m.generateWindowTableRowView(2)) //nolint: mnd - table.Render() - return buff.String() -} - -func (m *model) generateWindowInputHintView() string { - var hint string - switch m.currentCursor { - case pointToTruncateTo: - hint = `valid values are: -- M: truncate to month -- w: truncate to week -- d: truncate to day -- h: truncate to hour - -press (shift+up) or (shift+w) to increment value -press (shift+down) or (shift+s) to decrement value -` - case pointToOffset: - hint = `valid formats are: -- nMmh: example 1M2h meaning 1 month 2 hours -- mh: example 2h meaning 2 hours - -n can be negative, while m can be negative only if nM does NOT exist -` - case pointToSize: - hint = `valid formats are: -- nMmh: example 1M2h meaning 1 month 2 hours -- mh: example 2h meaning 2 hours - -both n and m can NOT be negative -` - case pointToYear: - hint = `year of the scheduled time - -press (shift+up) or (shift+w) to increment value -press (shift+down) or (shift+s) to decrement value -` - case pointToMonth: - hint = `month of the scheduled time - -press (shift+up) or (shift+w) to increment value -press (shift+down) or (shift+s) to decrement value -` - case pointToDay: - hint = `day of the scheduled time - -press (shift+up) or (shift+w) to increment value -press (shift+down) or (shift+s) to decrement value -` - case pointToHour: - hint = `hour of the scheduled time - -press (shift+up) or (shift+w) to increment value -press (shift+down) or (shift+s) to decrement value -` - case pointToMinute: - hint = `minute of the scheduled time - -press (shift+up) or (shift+w) to increment value -press (shift+down) or (shift+s) to decrement value -` - } - return hint -} - -func (m *model) generateWindowInputView() string { - buff := &bytes.Buffer{} - table := tablewriter.NewWriter(buff) - table.SetRowLine(true) - table.SetColumnAlignment([]int{tablewriter.ALIGN_LEFT, tablewriter.ALIGN_LEFT}) - table.Append([]string{ - "truncate_to", - m.generateValueWithCursorPointerView(pointToTruncateTo, string(m.truncateTo)), - }) - table.Append([]string{ - "offset", - m.generateValueWithCursorPointerView(pointToOffset, m.offsetInput.Value()), - }) - table.Append([]string{ - "size", - m.generateValueWithCursorPointerView(pointToSize, m.sizeInput.Value()), - }) - table.Append([]string{ - "job schedule", - m.generateSechduledTimeView(), - }) - table.Render() - return buff.String() -} - -func (m *model) generateWindowTableRowView(version int) []string { - window, err := models.NewWindow(version, string(m.truncateTo), m.offsetInput.Value(), m.sizeInput.Value()) - if err != nil { - return []string{fmt.Sprintf("%d", version), err.Error(), err.Error()} - } - - var startTimeRow string - if startTime, err := window.GetStartTime(m.scheduledTime); err != nil { - startTimeRow = err.Error() - } else { - startTimeRow = startTime.Format(time.RFC3339) - } - - var endTimeRow string - if endTime, err := window.GetEndTime(m.scheduledTime); err != nil { - endTimeRow = err.Error() - } else { - endTimeRow = endTime.Format(time.RFC3339) - } - return []string{fmt.Sprintf("%d", version), startTimeRow, endTimeRow} -} - -func (m *model) generateSechduledTimeView() string { - year := m.generateValueWithCursorPointerView(pointToYear, strconv.Itoa(m.scheduledTime.Year())) - month := m.generateValueWithCursorPointerView(pointToMonth, m.scheduledTime.Month().String()) - day := m.generateValueWithCursorPointerView(pointToDay, strconv.Itoa(m.scheduledTime.Day())) - hour := m.generateValueWithCursorPointerView(pointToHour, strconv.Itoa(m.scheduledTime.Hour())) - minute := m.generateValueWithCursorPointerView(pointToMinute, strconv.Itoa(m.scheduledTime.Minute())) - return year + " " + month + " " + day + " " + hour + ":" + minute -} - -func (m *model) generateValueWithCursorPointerView(targetCursor cursorPointer, value string) string { - if m.currentCursor == targetCursor { - var s strings.Builder - s.WriteString("[") - s.WriteString(value) - s.WriteString("]") - return s.String() - } - return value -} - -func (m *model) handleInput(msg tea.Msg) { - switch m.currentCursor { - case pointToOffset: - m.offsetInput, _ = m.offsetInput.Update(msg) - case pointToSize: - m.sizeInput, _ = m.sizeInput.Update(msg) - } -} - -func (m *model) handleDecrement() { - switch m.currentCursor { - case pointToTruncateTo: - m.decrementTruncateTo() - default: - m.decrementScheduledTime() - } -} - -func (m *model) decrementScheduledTime() { - switch m.currentCursor { - case pointToMinute: - m.scheduledTime = m.scheduledTime.Add(-1 * time.Minute) - case pointToHour: - m.scheduledTime = m.scheduledTime.Add(-1 * time.Hour) - case pointToDay: - m.scheduledTime = m.scheduledTime.AddDate(0, 0, -1) - case pointToMonth: - m.scheduledTime = m.scheduledTime.AddDate(0, -1, 0) - case pointToYear: - m.scheduledTime = m.scheduledTime.AddDate(-1, 0, 0) - } -} - -func (m *model) decrementTruncateTo() { - switch m.truncateTo { - case truncateToMonth: - m.truncateTo = truncateToWeek - case truncateToWeek: - m.truncateTo = truncateToDay - case truncateToDay: - m.truncateTo = truncateToHour - case truncateToHour: - m.truncateTo = truncateToMonth - } -} - -func (m *model) handleIncrement() { - switch m.currentCursor { - case pointToTruncateTo: - m.incrementTruncateTo() - default: - m.incrementScheduledTime() - } -} - -func (m *model) incrementScheduledTime() { - switch m.currentCursor { - case pointToMinute: - m.scheduledTime = m.scheduledTime.Add(time.Minute) - case pointToHour: - m.scheduledTime = m.scheduledTime.Add(time.Hour) - case pointToDay: - m.scheduledTime = m.scheduledTime.AddDate(0, 0, 1) - case pointToMonth: - m.scheduledTime = m.scheduledTime.AddDate(0, 1, 0) - case pointToYear: - m.scheduledTime = m.scheduledTime.AddDate(1, 0, 0) - } -} - -func (m *model) incrementTruncateTo() { - switch m.truncateTo { - case truncateToHour: - m.truncateTo = truncateToDay - case truncateToDay: - m.truncateTo = truncateToWeek - case truncateToWeek: - m.truncateTo = truncateToMonth - case truncateToMonth: - m.truncateTo = truncateToHour - } -} - -func (m *model) handleRight() { - switch m.currentCursor { - case pointToYear: - m.currentCursor = pointToMonth - case pointToMonth: - m.currentCursor = pointToDay - case pointToDay: - m.currentCursor = pointToHour - case pointToHour: - m.currentCursor = pointToMinute - case pointToMinute: - m.currentCursor = pointToYear - } -} - -func (m *model) handleLeft() { - switch m.currentCursor { - case pointToMinute: - m.currentCursor = pointToHour - case pointToHour: - m.currentCursor = pointToDay - case pointToDay: - m.currentCursor = pointToMonth - case pointToMonth: - m.currentCursor = pointToYear - case pointToYear: - m.currentCursor = pointToMinute - } -} - -func (m *model) handleDown() { - switch m.currentCursor { - case pointToTruncateTo: - m.offsetInput.Focus() - m.currentCursor = pointToOffset - case pointToOffset: - m.offsetInput.Blur() - m.sizeInput.Focus() - m.currentCursor = pointToSize - case pointToSize: - m.sizeInput.Blur() - m.currentCursor = pointToYear - default: - m.currentCursor = pointToTruncateTo - } -} - -func (m *model) handleUp() { - switch m.currentCursor { - case pointToTruncateTo: - m.currentCursor = pointToYear - case pointToOffset: - m.offsetInput.Blur() - m.currentCursor = pointToTruncateTo - case pointToSize: - m.sizeInput.Blur() - m.offsetInput.Focus() - m.currentCursor = pointToOffset - default: - m.sizeInput.Focus() - m.currentCursor = pointToSize - } -} diff --git a/client/cmd/playground/window/window.go b/client/cmd/playground/window/window.go index 8b5eaa240f..16b6c3c8ed 100644 --- a/client/cmd/playground/window/window.go +++ b/client/cmd/playground/window/window.go @@ -6,13 +6,10 @@ import ( "github.com/spf13/cobra" "github.com/goto/optimus/client/cmd/internal/logger" - "github.com/goto/optimus/client/cmd/playground/window/v1v2" ) type command struct { log log.Logger - - isV1V2 bool } // NewCommand initializes command for window playground @@ -24,15 +21,10 @@ func NewCommand() *cobra.Command { RunE: window.RunE, } - cmd.Flags().BoolVar(&window.isV1V2, "v1-v2", false, "if set, then playground will be for v1 and v2") return cmd } func (j *command) RunE(_ *cobra.Command, _ []string) error { - if j.isV1V2 { - return j.runV1V2() - } - return j.runV3() } @@ -60,29 +52,3 @@ the following keys | quit | : q or ctrl+c | p := tea.NewProgram(newModel()) return p.Start() } - -func (j *command) runV1V2() error { - message := ` - __ __ _ - \ \ / / | | - \ \ /\ / /___ | | ___ ___ _ __ ___ ___ - \ \/ \/ // _ \| | / __|/ _ \ | '_ ' _ \ / _ \ - \ /\ /| __/| || (__| (_) || | | | | || __/ - \/ \/ \___||_| \___|\___/ |_| |_| |_| \___| - - 🚨THIS VERSION IS SOON TO BE DEPRECATED🚨 - -Hi, this is an interactive CLI to play around with window configuration for v1 and v2. -You can navigate around the available configurations with the following keys: - ______________________________ - | up | : arrow up ↑ or w | - | down | : arrow down ↓ or s | - | right | : arrow right → or d | - | left | : arrow left ← or a | - | quit | : q or ctrl+c | - ------------------------------ -` - j.log.Info(message) - p := tea.NewProgram(v1v2.NewModel()) - return p.Start() -} diff --git a/client/local/model/job_spec.go b/client/local/model/job_spec.go index 690db96801..975e390d07 100644 --- a/client/local/model/job_spec.go +++ b/client/local/model/job_spec.go @@ -73,9 +73,7 @@ type JobSpecTask struct { } type JobSpecTaskWindow struct { - Size string `yaml:"size,omitempty"` - // deprecated, replaced by ShiftBy - Offset string `yaml:"offset,omitempty"` + Size string `yaml:"size,omitempty"` TruncateTo string `yaml:"truncate_to,omitempty"` Preset string `yaml:"preset,omitempty"` ShiftBy string `yaml:"shift_by,omitempty"` @@ -146,21 +144,13 @@ func (j *JobSpec) ToProto() *pb.JobSpecification { Labels: j.Labels, Behavior: j.getProtoJobSpecBehavior(), Metadata: j.getProtoJobMetadata(), - } - - if js.Version < NewWindowVersion { - js.WindowSize = j.Task.Window.Size - js.WindowOffset = j.Task.Window.Offset - js.WindowTruncateTo = j.Task.Window.TruncateTo - js.WindowPreset = j.Task.Window.Preset - } else { - js.Window = &pb.JobSpecification_Window{ + Window: &pb.JobSpecification_Window{ Preset: j.Task.Window.Preset, Size: j.Task.Window.Size, ShiftBy: j.Task.Window.ShiftBy, Location: j.Task.Window.Location, TruncateTo: j.Task.Window.TruncateTo, - } + }, } return js @@ -395,9 +385,11 @@ func (j *JobSpec) MergeFrom(anotherJobSpec *JobSpec) { } j.Task.Name = getValue(j.Task.Name, anotherJobSpec.Task.Name) - j.Task.Window.TruncateTo = getValue(j.Task.Window.TruncateTo, anotherJobSpec.Task.Window.TruncateTo) - j.Task.Window.Offset = getValue(j.Task.Window.Offset, anotherJobSpec.Task.Window.Offset) j.Task.Window.Size = getValue(j.Task.Window.Size, anotherJobSpec.Task.Window.Size) + j.Task.Window.TruncateTo = getValue(j.Task.Window.TruncateTo, anotherJobSpec.Task.Window.TruncateTo) + j.Task.Window.Preset = getValue(j.Task.Window.Preset, anotherJobSpec.Task.Window.Preset) + j.Task.Window.ShiftBy = getValue(j.Task.Window.ShiftBy, anotherJobSpec.Task.Window.ShiftBy) + j.Task.Window.Location = getValue(j.Task.Window.Location, anotherJobSpec.Task.Window.Location) if anotherJobSpec.Task.Config != nil { if j.Task.Config == nil { j.Task.Config = map[string]string{} @@ -483,23 +475,15 @@ func getValue[V int | string | bool | time.Duration](reference, other V) V { func ToJobSpec(protoSpec *pb.JobSpecification) *JobSpec { var w JobSpecTaskWindow - if protoSpec.Version < NewWindowVersion { - w = JobSpecTaskWindow{ - Size: protoSpec.WindowSize, - Offset: protoSpec.WindowOffset, - TruncateTo: protoSpec.WindowTruncateTo, - Preset: protoSpec.WindowPreset, - } - } else { - wc := protoSpec.Window - w = JobSpecTaskWindow{ - Size: wc.Size, - ShiftBy: wc.ShiftBy, - TruncateTo: wc.TruncateTo, - Location: wc.Location, - Preset: wc.Preset, - } + wc := protoSpec.Window + w = JobSpecTaskWindow{ + Size: wc.Size, + ShiftBy: wc.ShiftBy, + TruncateTo: wc.TruncateTo, + Location: wc.Location, + Preset: wc.Preset, } + return &JobSpec{ Version: int(protoSpec.Version), Name: protoSpec.Name, diff --git a/client/local/model/job_spec_test.go b/client/local/model/job_spec_test.go index 725209fe5d..d35474823b 100644 --- a/client/local/model/job_spec_test.go +++ b/client/local/model/job_spec_test.go @@ -117,9 +117,8 @@ func (*JobSpecTestSuite) getCompleteJobSpec() model.JobSpec { "taskkey": "taskvalue", }, Window: model.JobSpecTaskWindow{ - Size: "24h", - Offset: "1h", - TruncateTo: "d", + Size: "1d", + ShiftBy: "1h", }, }, Asset: map[string]string{ @@ -218,9 +217,10 @@ func (*JobSpecTestSuite) getCompleteJobSpecProto() *pb.JobSpecification { }, }, }, - WindowSize: "24h", - WindowOffset: "1h", - WindowTruncateTo: "d", + Window: &pb.JobSpecification_Window{ + Size: "1d", + ShiftBy: "1h", + }, Assets: map[string]string{ "query.sql": "SELECT * FROM example", }, diff --git a/core/job/handler/v1beta1/job.go b/core/job/handler/v1beta1/job.go index 1daab6ff9e..087aad0edf 100644 --- a/core/job/handler/v1beta1/job.go +++ b/core/job/handler/v1beta1/job.go @@ -10,13 +10,11 @@ import ( "github.com/goto/salt/log" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" - "google.golang.org/protobuf/types/known/timestamppb" "github.com/goto/optimus/core/job" "github.com/goto/optimus/core/job/dto" "github.com/goto/optimus/core/tenant" "github.com/goto/optimus/internal/errors" - "github.com/goto/optimus/internal/models" "github.com/goto/optimus/internal/utils/filter" "github.com/goto/optimus/internal/writer" "github.com/goto/optimus/plugin" @@ -433,45 +431,6 @@ func (jh *JobHandler) ListJobSpecification(ctx context.Context, req *pb.ListJobS }, merr } -func (jh *JobHandler) GetWindow(_ context.Context, req *pb.GetWindowRequest) (*pb.GetWindowResponse, error) { - // TODO: the default version to be deprecated & made mandatory in future releases - version := 1 - if err := req.GetScheduledAt().CheckValid(); err != nil { - jh.l.Error("scheduled at is invalid: %s", err) - return nil, fmt.Errorf("%w: failed to parse schedule time %s", err, req.GetScheduledAt()) - } - - if req.Version != 0 { - version = int(req.Version) - } - window, err := models.NewWindow(version, req.GetTruncateTo(), req.GetOffset(), req.GetSize()) - if err != nil { - jh.l.Error("error initializing window with version [%d]: %s", req.Version, err) - return nil, err - } - if err := window.Validate(); err != nil { - jh.l.Error("error validating window: %s", err) - return nil, err - } - - me := errors.NewMultiError("get window errors") - - startTime, err := window.GetStartTime(req.GetScheduledAt().AsTime()) - me.Append(err) - - endTime, err := window.GetEndTime(req.GetScheduledAt().AsTime()) - me.Append(err) - - if len(me.Errors) > 0 { - return nil, me - } - - return &pb.GetWindowResponse{ - Start: timestamppb.New(startTime), - End: timestamppb.New(endTime), - }, nil -} - func (jh *JobHandler) ReplaceAllJobSpecifications(stream pb.JobSpecificationService_ReplaceAllJobSpecificationsServer) error { responseWriter := writer.NewReplaceAllJobSpecificationsResponseWriter(stream) var errNamespaces []string diff --git a/core/job/handler/v1beta1/job_adapter.go b/core/job/handler/v1beta1/job_adapter.go index ed01855af2..9b7b9d205a 100644 --- a/core/job/handler/v1beta1/job_adapter.go +++ b/core/job/handler/v1beta1/job_adapter.go @@ -10,7 +10,6 @@ import ( "github.com/goto/optimus/internal/errors" "github.com/goto/optimus/internal/lib/labels" "github.com/goto/optimus/internal/lib/window" - "github.com/goto/optimus/internal/models" "github.com/goto/optimus/internal/utils" pb "github.com/goto/optimus/protos/gotocompany/optimus/core/v1beta1" ) @@ -44,21 +43,13 @@ func ToJobProto(jobEntity *job.Job) *pb.JobSpecification { Metadata: fromMetadata(spec.Metadata()), Destination: jobEntity.Destination().String(), Sources: fromResourceURNs(jobEntity.Sources()), - } - - if spec.Version() == window.NewWindowVersion { - j.Window = &pb.JobSpecification_Window{ + Window: &pb.JobSpecification_Window{ Preset: spec.WindowConfig().Preset, Size: spec.WindowConfig().GetSimpleConfig().Size, ShiftBy: spec.WindowConfig().GetSimpleConfig().ShiftBy, Location: spec.WindowConfig().GetSimpleConfig().Location, TruncateTo: spec.WindowConfig().GetSimpleConfig().TruncateTo, - } - } else { - j.WindowPreset = spec.WindowConfig().Preset - j.WindowSize = spec.WindowConfig().GetSize() - j.WindowOffset = spec.WindowConfig().GetOffset() - j.WindowTruncateTo = spec.WindowConfig().GetTruncateTo() + }, } return j @@ -263,20 +254,6 @@ func toWindow(js *pb.JobSpecification) (window.Config, error) { } } - if js.WindowPreset != "" { - return window.NewPresetConfig(js.WindowPreset) - } - - if js.Version < window.NewWindowVersion { - w, err := models.NewWindow(int(js.Version), js.WindowTruncateTo, js.WindowOffset, js.WindowSize) - if err != nil { - return window.Config{}, err - } - if err := w.Validate(); err != nil { - return window.Config{}, err - } - return window.NewCustomConfig(w), nil - } return window.NewIncrementalConfig(), nil } diff --git a/core/job/handler/v1beta1/job_test.go b/core/job/handler/v1beta1/job_test.go index 0d829e52a0..b2a36a19b4 100644 --- a/core/job/handler/v1beta1/job_test.go +++ b/core/job/handler/v1beta1/job_test.go @@ -5,13 +5,11 @@ import ( "errors" "io" "testing" - "time" "github.com/goto/salt/log" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "google.golang.org/grpc/metadata" - "google.golang.org/protobuf/types/known/timestamppb" "github.com/goto/optimus/core/job" "github.com/goto/optimus/core/job/dto" @@ -19,7 +17,6 @@ import ( "github.com/goto/optimus/core/resource" "github.com/goto/optimus/core/tenant" "github.com/goto/optimus/internal/lib/window" - "github.com/goto/optimus/internal/models" "github.com/goto/optimus/internal/utils/filter" "github.com/goto/optimus/internal/writer" "github.com/goto/optimus/plugin" @@ -44,9 +41,8 @@ func TestNewJobHandler(t *testing.T) { assert.NoError(t, err) jobSchedule, err := job.NewScheduleBuilder(startDate).Build() assert.NoError(t, err) - w, err := models.NewWindow(jobVersion, "d", "24h", "24h") + w, err := window.NewConfig("1d", "1d", "", "") assert.NoError(t, err) - jobWindow := window.NewCustomConfig(w) jobConfig, err := job.ConfigFrom(map[string]string{"sample_key": "sample_value"}) assert.NoError(t, err) jobTask := job.NewTask("bq2bq", jobConfig, "") @@ -102,12 +98,13 @@ func TestNewJobHandler(t *testing.T) { EndDate: jobSchedule.EndDate().String(), Interval: jobSchedule.Interval(), Task: &pb.JobSpecTask{ - Name: jobTask.Name().String(), - Version: jobTask.Version(), + Name: jobTask.Name().String(), Version: jobTask.Version(), + }, + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), } jobProtos := []*pb.JobSpecification{jobSpecProto} request := pb.AddJobSpecificationsRequest{ @@ -140,15 +137,16 @@ func TestNewJobHandler(t *testing.T) { EndDate: jobSchedule.EndDate().String(), Interval: jobSchedule.Interval(), Task: &pb.JobSpecTask{ - Name: jobTask.Name().String(), - Version: jobTask.Version(), + Name: jobTask.Name().String(), Version: jobTask.Version(), + }, + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), - Behavior: jobBehavior, - Dependencies: jobDependencies, - Metadata: jobMetadata, + Behavior: jobBehavior, + Dependencies: jobDependencies, + Metadata: jobMetadata, } jobProtos := []*pb.JobSpecification{jobSpecProto} request := pb.AddJobSpecificationsRequest{ @@ -196,12 +194,13 @@ func TestNewJobHandler(t *testing.T) { EndDate: jobSchedule.EndDate().String(), Interval: jobSchedule.Interval(), Task: &pb.JobSpecTask{ - Name: jobTask.Name().String(), - Version: jobTask.Version(), + Name: jobTask.Name().String(), Version: jobTask.Version(), + }, + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), }, { Version: int32(jobVersion), @@ -213,9 +212,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, }, } request := pb.AddJobSpecificationsRequest{ @@ -252,9 +253,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, }, { Version: int32(jobVersion), @@ -266,9 +269,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, }, } request := pb.AddJobSpecificationsRequest{ @@ -304,9 +309,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, }, { Version: int32(jobVersion), @@ -318,9 +325,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, }, } request := pb.AddJobSpecificationsRequest{ @@ -363,10 +372,12 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), - Behavior: behaviorWithInvalidAlertConf, + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, + Behavior: behaviorWithInvalidAlertConf, }, { Version: int32(jobVersion), @@ -378,9 +389,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, }, } request := pb.AddJobSpecificationsRequest{ @@ -416,9 +429,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, }, } request := pb.AddJobSpecificationsRequest{ @@ -448,9 +463,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, }, { Version: int32(jobVersion), @@ -461,9 +478,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, }, } request := pb.AddJobSpecificationsRequest{ @@ -496,9 +515,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, } jobProtos := []*pb.JobSpecification{jobSpecProto} request := pb.UpdateJobSpecificationsRequest{ @@ -532,12 +553,14 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), - Behavior: jobBehavior, - Dependencies: jobDependencies, - Metadata: jobMetadata, + Behavior: jobBehavior, + Dependencies: jobDependencies, + Metadata: jobMetadata, + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, } jobProtos := []*pb.JobSpecification{jobSpecProto} request := pb.UpdateJobSpecificationsRequest{ @@ -585,9 +608,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, }, { Version: int32(jobVersion), @@ -599,9 +624,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, }, } request := pb.UpdateJobSpecificationsRequest{ @@ -632,9 +659,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, }, } request := pb.UpdateJobSpecificationsRequest{ @@ -666,9 +695,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, }, { Version: int32(jobVersion), @@ -679,9 +710,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, }, } request := pb.UpdateJobSpecificationsRequest{ @@ -714,9 +747,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, } jobProtos := []*pb.JobSpecification{jobSpecProto} request := pb.UpsertJobSpecificationsRequest{ @@ -754,12 +789,15 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), - Behavior: jobBehavior, - Dependencies: jobDependencies, - Metadata: jobMetadata, + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, + + Behavior: jobBehavior, + Dependencies: jobDependencies, + Metadata: jobMetadata, } jobProtos := []*pb.JobSpecification{jobSpecProto} request := pb.UpsertJobSpecificationsRequest{ @@ -815,9 +853,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, }, { Version: int32(jobVersion), @@ -829,9 +869,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, }, } request := pb.UpsertJobSpecificationsRequest{ @@ -868,9 +910,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, }, } request := pb.UpsertJobSpecificationsRequest{ @@ -904,9 +948,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, }, { Version: int32(jobVersion), @@ -918,9 +964,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, }, } request := pb.UpsertJobSpecificationsRequest{ @@ -1189,68 +1237,6 @@ func TestNewJobHandler(t *testing.T) { assert.Nil(t, resp) }) }) - t.Run("GetWindow", func(t *testing.T) { - t.Run("returns error if scheduledAt is not valid", func(t *testing.T) { - req := &pb.GetWindowRequest{ - ScheduledAt: nil, - } - jobHandler := v1beta1.NewJobHandler(nil, nil, log) - - resp, err := jobHandler.GetWindow(ctx, req) - assert.Error(t, err) - assert.Nil(t, resp) - }) - t.Run("returns error if version is not valid", func(t *testing.T) { - req := &pb.GetWindowRequest{ - Version: 3, - ScheduledAt: timestamppb.New(time.Date(2022, 11, 18, 13, 0, 0, 0, time.UTC)), - } - jobHandler := v1beta1.NewJobHandler(nil, nil, log) - - resp, err := jobHandler.GetWindow(ctx, req) - assert.Error(t, err) - assert.Nil(t, resp) - }) - t.Run("returns error if window is not valid", func(t *testing.T) { - req := &pb.GetWindowRequest{ - Version: 2, - ScheduledAt: timestamppb.New(time.Date(2022, 11, 18, 13, 0, 0, 0, time.UTC)), - Size: "1", - } - jobHandler := v1beta1.NewJobHandler(nil, nil, log) - - resp, err := jobHandler.GetWindow(ctx, req) - assert.Error(t, err) - assert.Nil(t, resp) - }) - t.Run("returns dstart and dend", func(t *testing.T) { - req := &pb.GetWindowRequest{ - Version: 2, - ScheduledAt: timestamppb.New(time.Date(2022, 11, 18, 13, 0, 0, 0, time.UTC)), - Size: "24h", - Offset: "0", - TruncateTo: "d", - } - jobHandler := v1beta1.NewJobHandler(nil, nil, log) - - resp, err := jobHandler.GetWindow(ctx, req) - assert.NoError(t, err) - assert.NotNil(t, resp) - }) - t.Run("should default to version 1 if not provided", func(t *testing.T) { - req := &pb.GetWindowRequest{ - ScheduledAt: timestamppb.New(time.Date(2022, 11, 18, 13, 0, 0, 0, time.UTC)), - Size: "24h", - Offset: "0", - TruncateTo: "d", - } - jobHandler := v1beta1.NewJobHandler(nil, nil, log) - - resp, err := jobHandler.GetWindow(ctx, req) - assert.NoError(t, err) - assert.NotNil(t, resp) - }) - }) t.Run("ReplaceAllJobSpecifications", func(t *testing.T) { var jobNamesWithInvalidSpec []job.Name t.Run("replaces all job specifications of a tenant", func(t *testing.T) { @@ -1270,17 +1256,18 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), - Config: []*pb.JobConfigItem{}, - Window: &pb.JobSpecification_Window{}, - Dependencies: []*pb.JobDependency{}, - Assets: map[string]string{}, - Hooks: []*pb.JobSpecHook{}, - Description: "", - Labels: map[string]string{}, - Behavior: &pb.JobSpecification_Behavior{Notify: []*pb.JobSpecification_Behavior_Notifiers{}}, + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, + Config: []*pb.JobConfigItem{}, + Dependencies: []*pb.JobDependency{}, + Assets: map[string]string{}, + Hooks: []*pb.JobSpecHook{}, + Description: "", + Labels: map[string]string{}, + Behavior: &pb.JobSpecification_Behavior{Notify: []*pb.JobSpecification_Behavior_Notifiers{}}, Metadata: &pb.JobMetadata{ Resource: &pb.JobSpecMetadataResource{ Request: nil, @@ -1301,9 +1288,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, }, } request := &pb.ReplaceAllJobSpecificationsRequest{ @@ -1341,9 +1330,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, }, } request1 := &pb.ReplaceAllJobSpecificationsRequest{ @@ -1395,9 +1386,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, }, } request := &pb.ReplaceAllJobSpecificationsRequest{ @@ -1435,9 +1428,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, }, { Version: int32(jobVersion), @@ -1449,9 +1444,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, }, } request1 := &pb.ReplaceAllJobSpecificationsRequest{} @@ -1491,9 +1488,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, }, { Version: int32(jobVersion), @@ -1505,9 +1504,11 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, }, } request := &pb.ReplaceAllJobSpecificationsRequest{ @@ -1641,7 +1642,7 @@ func TestNewJobHandler(t *testing.T) { changeLogService := new(ChangeLogService) defer jobService.AssertExpectations(t) - specA, _ := job.NewSpecBuilder(jobVersion, "job-A", sampleOwner, jobSchedule, jobWindow, jobTask).Build() + specA, _ := job.NewSpecBuilder(jobVersion, "job-A", sampleOwner, jobSchedule, w, jobTask).Build() jobA := job.NewJob(sampleTenant, specA, resourceURNA, []resource.URN{resourceURNB}, false) request := pb.GetJobSpecificationRequest{ @@ -1682,9 +1683,9 @@ func TestNewJobHandler(t *testing.T) { request := pb.GetJobSpecificationsRequest{} - specA, _ := job.NewSpecBuilder(jobVersion, "job-A", sampleOwner, jobSchedule, jobWindow, jobTask).WithAsset(asset).Build() + specA, _ := job.NewSpecBuilder(jobVersion, "job-A", sampleOwner, jobSchedule, w, jobTask).WithAsset(asset).Build() jobA := job.NewJob(sampleTenant, specA, resourceURNA, []resource.URN{resourceURNB}, false) - specB, _ := job.NewSpecBuilder(jobVersion, "job-B", sampleOwner, jobSchedule, jobWindow, jobTask).WithAsset(asset).Build() + specB, _ := job.NewSpecBuilder(jobVersion, "job-B", sampleOwner, jobSchedule, w, jobTask).WithAsset(asset).Build() jobB := job.NewJob(sampleTenant, specB, resourceURNB, []resource.URN{resourceURNC}, false) jobService.On("GetByFilter", ctx, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]*job.Job{jobA, jobB}, nil) @@ -1706,9 +1707,9 @@ func TestNewJobHandler(t *testing.T) { IgnoreAssets: true, } - specA, _ := job.NewSpecBuilder(jobVersion, "job-A", sampleOwner, jobSchedule, jobWindow, jobTask).WithAsset(asset).Build() + specA, _ := job.NewSpecBuilder(jobVersion, "job-A", sampleOwner, jobSchedule, w, jobTask).WithAsset(asset).Build() jobA := job.NewJob(sampleTenant, specA, resourceURNA, []resource.URN{resourceURNB}, false) - specB, _ := job.NewSpecBuilder(jobVersion, "job-B", sampleOwner, jobSchedule, jobWindow, jobTask).WithAsset(asset).Build() + specB, _ := job.NewSpecBuilder(jobVersion, "job-B", sampleOwner, jobSchedule, w, jobTask).WithAsset(asset).Build() jobB := job.NewJob(sampleTenant, specB, resourceURNB, []resource.URN{resourceURNC}, false) jobService.On("GetByFilter", ctx, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]*job.Job{jobA, jobB}, nil) @@ -1750,9 +1751,9 @@ func TestNewJobHandler(t *testing.T) { NamespaceName: sampleTenant.NamespaceName().String(), } - specA, _ := job.NewSpecBuilder(jobVersion, "job-A", sampleOwner, jobSchedule, jobWindow, jobTask).WithAsset(asset).Build() + specA, _ := job.NewSpecBuilder(jobVersion, "job-A", sampleOwner, jobSchedule, w, jobTask).WithAsset(asset).Build() jobA := job.NewJob(sampleTenant, specA, resourceURNA, []resource.URN{resourceURNB}, false) - specB, _ := job.NewSpecBuilder(jobVersion, "job-B", sampleOwner, jobSchedule, jobWindow, jobTask).WithAsset(asset).Build() + specB, _ := job.NewSpecBuilder(jobVersion, "job-B", sampleOwner, jobSchedule, w, jobTask).WithAsset(asset).Build() jobB := job.NewJob(sampleTenant, specB, resourceURNB, []resource.URN{resourceURNC}, false) jobService.On("GetByFilter", ctx, mock.Anything, mock.Anything).Return([]*job.Job{jobA, jobB}, nil) @@ -1776,9 +1777,9 @@ func TestNewJobHandler(t *testing.T) { IgnoreAssets: true, } - specA, _ := job.NewSpecBuilder(jobVersion, "job-A", sampleOwner, jobSchedule, jobWindow, jobTask).WithAsset(asset).Build() + specA, _ := job.NewSpecBuilder(jobVersion, "job-A", sampleOwner, jobSchedule, w, jobTask).WithAsset(asset).Build() jobA := job.NewJob(sampleTenant, specA, resourceURNA, []resource.URN{resourceURNB}, false) - specB, _ := job.NewSpecBuilder(jobVersion, "job-B", sampleOwner, jobSchedule, jobWindow, jobTask).WithAsset(asset).Build() + specB, _ := job.NewSpecBuilder(jobVersion, "job-B", sampleOwner, jobSchedule, w, jobTask).WithAsset(asset).Build() jobB := job.NewJob(sampleTenant, specB, resourceURNB, []resource.URN{resourceURNC}, false) jobService.On("GetByFilter", ctx, mock.Anything, mock.Anything).Return([]*job.Job{jobA, jobB}, nil) @@ -1805,7 +1806,7 @@ func TestNewJobHandler(t *testing.T) { httpUpstream, _ := job.NewSpecHTTPUpstreamBuilder("sample-upstream", "sample-url").Build() upstreamSpec, _ := job.NewSpecUpstreamBuilder().WithSpecHTTPUpstream([]*job.SpecHTTPUpstream{httpUpstream}).Build() - specA, _ := job.NewSpecBuilder(jobVersion, "job-A", sampleOwner, jobSchedule, jobWindow, jobTask).WithSpecUpstream(upstreamSpec).WithMetadata(metadataSpec).Build() + specA, _ := job.NewSpecBuilder(jobVersion, "job-A", sampleOwner, jobSchedule, w, jobTask).WithSpecUpstream(upstreamSpec).WithMetadata(metadataSpec).Build() jobA := job.NewJob(sampleTenant, specA, resourceURNA, nil, false) upstreamB := job.NewUpstreamResolved("job-B", "", resourceURNB, sampleTenant, "static", "bq2bq", false) @@ -1848,11 +1849,13 @@ func TestNewJobHandler(t *testing.T) { Value: "sample_value", }}, }, - TaskName: specA.Task().Name().String(), - WindowSize: specA.WindowConfig().GetSize(), - WindowOffset: specA.WindowConfig().GetOffset(), - WindowTruncateTo: specA.WindowConfig().GetTruncateTo(), - Destination: "store://table-A", + TaskName: specA.Task().Name().String(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, + Destination: "store://table-A", Config: []*pb.JobConfigItem{{ Name: "sample_key", Value: "sample_value", @@ -1923,7 +1926,7 @@ func TestNewJobHandler(t *testing.T) { hook1, _ := job.NewHook("hook-1", jobConfig, "") - specA, _ := job.NewSpecBuilder(jobVersion, "job-A", sampleOwner, jobSchedule, jobWindow, jobTask). + specA, _ := job.NewSpecBuilder(jobVersion, "job-A", sampleOwner, jobSchedule, w, jobTask). WithSpecUpstream(upstreamSpec). WithHooks([]*job.Hook{hook1}).Build() jobA := job.NewJob(sampleTenant, specA, resourceURNA, nil, false) @@ -1966,14 +1969,17 @@ func TestNewJobHandler(t *testing.T) { Task: &pb.JobSpecTask{ Name: jobTask.Name().String(), }, - TaskName: jobTask.Name().String(), - WindowSize: jobWindow.GetSize(), - WindowOffset: jobWindow.GetOffset(), - WindowTruncateTo: jobWindow.GetTruncateTo(), - Behavior: jobBehavior, - Dependencies: jobDependenciesWithHTTPProto, - Metadata: jobMetadata, - Hooks: jobHooksProto, + TaskName: jobTask.Name().String(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, + + Behavior: jobBehavior, + Dependencies: jobDependenciesWithHTTPProto, + Metadata: jobMetadata, + Hooks: jobHooksProto, } req := &pb.JobInspectRequest{ ProjectName: project.Name().String(), @@ -1996,14 +2002,16 @@ func TestNewJobHandler(t *testing.T) { Name: specA.Task().Name().String(), Config: configs, }, - TaskName: specA.Task().Name().String(), - WindowSize: specA.WindowConfig().GetSize(), - WindowOffset: specA.WindowConfig().GetOffset(), - WindowTruncateTo: specA.WindowConfig().GetTruncateTo(), - Destination: "store://table-A", - Config: configs, - Dependencies: jobDependenciesWithHTTPProto, - Hooks: jobHooksProto, + TaskName: specA.Task().Name().String(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, + Destination: "store://table-A", + Config: configs, + Dependencies: jobDependenciesWithHTTPProto, + Hooks: jobHooksProto, }, Destination: "store://table-A", }, @@ -2106,7 +2114,7 @@ func TestNewJobHandler(t *testing.T) { jobService := new(JobService) changeLogService := new(ChangeLogService) - specA, _ := job.NewSpecBuilder(jobVersion, "job-A", sampleOwner, jobSchedule, jobWindow, jobTask).Build() + specA, _ := job.NewSpecBuilder(jobVersion, "job-A", sampleOwner, jobSchedule, w, jobTask).Build() jobA := job.NewJob(sampleTenant, specA, resourceURNA, nil, false) upstreamB := job.NewUpstreamResolved("job-B", "", resourceURNB, sampleTenant, "static", "bq2bq", false) @@ -2153,11 +2161,13 @@ func TestNewJobHandler(t *testing.T) { Value: "sample_value", }}, }, - TaskName: specA.Task().Name().String(), - WindowSize: specA.WindowConfig().GetSize(), - WindowOffset: specA.WindowConfig().GetOffset(), - WindowTruncateTo: specA.WindowConfig().GetTruncateTo(), - Destination: "store://table-A", + TaskName: specA.Task().Name().String(), + Window: &pb.JobSpecification_Window{ + Size: w.GetSimpleConfig().Size, + ShiftBy: w.GetSimpleConfig().ShiftBy, + TruncateTo: w.GetSimpleConfig().TruncateTo, + }, + Destination: "store://table-A", Config: []*pb.JobConfigItem{{ Name: "sample_key", Value: "sample_value", @@ -2210,7 +2220,7 @@ func TestNewJobHandler(t *testing.T) { httpUpstream, _ := job.NewSpecHTTPUpstreamBuilder("sample-upstream", "sample-url").Build() upstreamSpec, _ := job.NewSpecUpstreamBuilder().WithSpecHTTPUpstream([]*job.SpecHTTPUpstream{httpUpstream}).Build() - specA, _ := job.NewSpecBuilder(jobVersion, "job-A", sampleOwner, jobSchedule, jobWindow, jobTask).WithSpecUpstream(upstreamSpec).Build() + specA, _ := job.NewSpecBuilder(jobVersion, "job-A", sampleOwner, jobSchedule, w, jobTask).WithSpecUpstream(upstreamSpec).Build() basicInfoLogger := writer.BufferedLogger{Messages: []*pb.Log{ {Message: "not found"}, @@ -2281,7 +2291,7 @@ func TestNewJobHandler(t *testing.T) { changeLogService := new(ChangeLogService) defer jobService.AssertExpectations(t) - specA, _ := job.NewSpecBuilder(jobVersion, "job-A", sampleOwner, jobSchedule, jobWindow, jobTask).Build() + specA, _ := job.NewSpecBuilder(jobVersion, "job-A", sampleOwner, jobSchedule, w, jobTask).Build() jobA := job.NewJob(sampleTenant, specA, resourceURNA, []resource.URN{resourceURNB}, false) req := &pb.GetJobTaskRequest{ @@ -2302,7 +2312,7 @@ func TestNewJobHandler(t *testing.T) { changeLogService := new(ChangeLogService) defer jobService.AssertExpectations(t) - specA, _ := job.NewSpecBuilder(jobVersion, "job-A", sampleOwner, jobSchedule, jobWindow, jobTask).Build() + specA, _ := job.NewSpecBuilder(jobVersion, "job-A", sampleOwner, jobSchedule, w, jobTask).Build() jobA := job.NewJob(sampleTenant, specA, resourceURNA, []resource.URN{resourceURNB}, false) req := &pb.GetJobTaskRequest{ diff --git a/core/job/job_test.go b/core/job/job_test.go index faf96340cb..6f147b04f7 100644 --- a/core/job/job_test.go +++ b/core/job/job_test.go @@ -9,7 +9,6 @@ import ( "github.com/goto/optimus/core/resource" "github.com/goto/optimus/core/tenant" "github.com/goto/optimus/internal/lib/window" - "github.com/goto/optimus/internal/models" ) func TestEntityJob(t *testing.T) { @@ -27,8 +26,7 @@ func TestEntityJob(t *testing.T) { jobVersion := 1 startDate, _ := job.ScheduleDateFrom("2022-10-01") jobSchedule, _ := job.NewScheduleBuilder(startDate).Build() - w, _ := models.NewWindow(jobVersion, "d", "24h", "24h") - jobWindow := window.NewCustomConfig(w) + jobWindow, _ := window.NewConfig("1d", "1d", "", "") jobTaskConfig, _ := job.ConfigFrom(map[string]string{"sample_task_key": "sample_value"}) jobTask := job.NewTask("bq2bq", jobTaskConfig, "") diff --git a/core/job/resolver/external_upstream_resolver_test.go b/core/job/resolver/external_upstream_resolver_test.go index 9bb5b2a84e..db58af48d8 100644 --- a/core/job/resolver/external_upstream_resolver_test.go +++ b/core/job/resolver/external_upstream_resolver_test.go @@ -16,7 +16,6 @@ import ( "github.com/goto/optimus/core/tenant" "github.com/goto/optimus/ext/resourcemanager" "github.com/goto/optimus/internal/lib/window" - "github.com/goto/optimus/internal/models" ) func TestExternalUpstreamResolver(t *testing.T) { @@ -29,8 +28,7 @@ func TestExternalUpstreamResolver(t *testing.T) { jobVersion := 1 startDate, _ := job.ScheduleDateFrom("2022-10-01") jobSchedule, _ := job.NewScheduleBuilder(startDate).Build() - w, _ := models.NewWindow(jobVersion, "d", "24h", "24h") - jobWindow := window.NewCustomConfig(w) + jobWindow, _ := window.NewConfig("1d", "1d", "", "") taskName, _ := job.TaskNameFrom("sample-task") jobTaskConfig, _ := job.ConfigFrom(map[string]string{"sample_task_key": "sample_value"}) jobTask := job.NewTask(taskName, jobTaskConfig, "") diff --git a/core/job/resolver/internal_upstream_resolver_test.go b/core/job/resolver/internal_upstream_resolver_test.go index 131c8cf2b5..fe8c5128a5 100644 --- a/core/job/resolver/internal_upstream_resolver_test.go +++ b/core/job/resolver/internal_upstream_resolver_test.go @@ -12,7 +12,6 @@ import ( "github.com/goto/optimus/core/resource" "github.com/goto/optimus/core/tenant" "github.com/goto/optimus/internal/lib/window" - "github.com/goto/optimus/internal/models" ) func TestInternalUpstreamResolver(t *testing.T) { @@ -35,8 +34,7 @@ func TestInternalUpstreamResolver(t *testing.T) { jobVersion := 1 startDate, _ := job.ScheduleDateFrom("2022-10-01") jobSchedule, _ := job.NewScheduleBuilder(startDate).Build() - w, _ := models.NewWindow(jobVersion, "d", "24h", "24h") - jobWindow := window.NewCustomConfig(w) + jobWindow, _ := window.NewConfig("1d", "1d", "", "") taskName, _ := job.TaskNameFrom("sample-task") jobTaskConfig := map[string]string{"sample_task_key": "sample_value"} jobTask := job.NewTask(taskName, jobTaskConfig, "") diff --git a/core/job/resolver/upstream_resolver_test.go b/core/job/resolver/upstream_resolver_test.go index 94cc9aa298..6c9ab2ebd8 100644 --- a/core/job/resolver/upstream_resolver_test.go +++ b/core/job/resolver/upstream_resolver_test.go @@ -13,7 +13,6 @@ import ( "github.com/goto/optimus/core/resource" "github.com/goto/optimus/core/tenant" "github.com/goto/optimus/internal/lib/window" - "github.com/goto/optimus/internal/models" "github.com/goto/optimus/internal/writer" ) @@ -36,9 +35,8 @@ func TestUpstreamResolver(t *testing.T) { assert.NoError(t, err) jobSchedule, err := job.NewScheduleBuilder(startDate).Build() assert.NoError(t, err) - w, err := models.NewWindow(jobVersion, "d", "24h", "24h") + jobWindow, err := window.NewConfig("1d", "1d", "", "") assert.NoError(t, err) - jobWindow := window.NewCustomConfig(w) jobTaskConfig, err := job.ConfigFrom(map[string]string{"sample_task_key": "sample_value"}) assert.NoError(t, err) taskName, _ := job.TaskNameFrom("sample-task") diff --git a/core/job/service/job_service.go b/core/job/service/job_service.go index 408042c08e..e2a36de342 100644 --- a/core/job/service/job_service.go +++ b/core/job/service/job_service.go @@ -147,7 +147,7 @@ type UpstreamResolver interface { } type JobRunInputCompiler interface { - Compile(ctx context.Context, job *scheduler.JobWithDetails, config scheduler.RunConfig, executedAt time.Time) (*scheduler.ExecutorInput, error) + Compile(ctx context.Context, job *scheduler.JobWithDetails, config scheduler.RunConfig) (*scheduler.ExecutorInput, error) } type ResourceExistenceChecker interface { @@ -1851,7 +1851,7 @@ func (j *JobService) validateRun(ctx context.Context, subjectJob *job.Job, desti jobWithDetails := j.getSchedulerJobWithDetail(subjectJob, destination) for _, config := range runConfigs { var msg string - if _, err := j.jobRunInputCompiler.Compile(ctx, jobWithDetails, config, referenceTime); err != nil { + if _, err := j.jobRunInputCompiler.Compile(ctx, jobWithDetails, config); err != nil { success = false msg = fmt.Sprintf("compiling [%s] with type [%s] failed with error: %v", config.Executor.Name, config.Executor.Type.String(), err) diff --git a/core/job/service/job_service_test.go b/core/job/service/job_service_test.go index 5d2b3bf523..bfb4fd0064 100644 --- a/core/job/service/job_service_test.go +++ b/core/job/service/job_service_test.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "testing" - "time" "github.com/goto/salt/log" "github.com/stretchr/testify/assert" @@ -21,7 +20,6 @@ import ( "github.com/goto/optimus/internal/compiler" optErrors "github.com/goto/optimus/internal/errors" "github.com/goto/optimus/internal/lib/window" - "github.com/goto/optimus/internal/models" "github.com/goto/optimus/internal/utils/filter" "github.com/goto/optimus/internal/writer" "github.com/goto/optimus/plugin" @@ -59,8 +57,8 @@ func TestJobService(t *testing.T) { assert.NoError(t, err) jobSchedule, err := job.NewScheduleBuilder(startDate).Build() assert.NoError(t, err) - w, _ := models.NewWindow(jobVersion, "d", "24h", "24h") - jobWindow := window.NewCustomConfig(w) + jobWindow, err := window.NewConfig("1d", "1d", "", "") + assert.NoError(t, err) jobTaskConfig, err := job.ConfigFrom(map[string]string{ "sample_task_key": "sample_value", "BQ_SERVICE_ACCOUNT": "service_account", @@ -2162,8 +2160,8 @@ func TestJobService(t *testing.T) { incomingSpecs := []*job.Spec{specA} - w2, _ := models.NewWindow(jobVersion, "d", "0h", "24h") - existingJobWindow := window.NewCustomConfig(w2) + existingJobWindow, err := window.NewConfig("1d", "", "", "") + assert.NoError(t, err) existingSpecA, _ := job.NewSpecBuilder(jobVersion, "job-A", "sample-owner", jobSchedule, existingJobWindow, jobTask).WithAsset(jobAsset).Build() existingJobA := job.NewJob(sampleTenant, existingSpecA, jobADestination, jobAUpstreamName, false) existingSpecs := []*job.Job{existingJobA} @@ -2198,7 +2196,7 @@ func TestJobService(t *testing.T) { jobService := service.NewJobService(jobRepo, upstreamRepo, downstreamRepo, pluginService, upstreamResolver, tenantDetailsGetter, eventHandler, log, jobDeploymentService, compiler.NewEngine(), nil, nil, alertManager) - err := jobService.ReplaceAll(ctx, sampleTenant, incomingSpecs, jobNamesWithInvalidSpec, logWriter) + err = jobService.ReplaceAll(ctx, sampleTenant, incomingSpecs, jobNamesWithInvalidSpec, logWriter) assert.NoError(t, err) }) t.Run("deletes the removed jobs", func(t *testing.T) { @@ -2361,8 +2359,8 @@ func TestJobService(t *testing.T) { specB, _ := job.NewSpecBuilder(jobVersion, "job-B", "sample-owner", jobSchedule, jobWindow, jobTask).WithAsset(jobAsset).Build() incomingSpecs := []*job.Spec{specA, specB} - w2, _ := models.NewWindow(jobVersion, "d", "0h", "24h") - existingJobWindow := window.NewCustomConfig(w2) + existingJobWindow, err := window.NewConfig("1d", "", "", "") + assert.NoError(t, err) existingSpecB, _ := job.NewSpecBuilder(jobVersion, "job-B", "sample-owner", jobSchedule, existingJobWindow, jobTask).WithAsset(jobAsset).Build() existingJobB := job.NewJob(sampleTenant, existingSpecB, resource.ZeroURN(), nil, false) existingSpecC, _ := job.NewSpecBuilder(jobVersion, "job-C", "sample-owner", jobSchedule, jobWindow, jobTask).WithAsset(jobAsset).Build() @@ -2418,7 +2416,7 @@ func TestJobService(t *testing.T) { jobDeploymentService.On("UploadJobs", ctx, sampleTenant, mock.Anything, emptyStringArr).Return(nil).Once() jobService := service.NewJobService(jobRepo, upstreamRepo, downstreamRepo, pluginService, upstreamResolver, tenantDetailsGetter, eventHandler, log, jobDeploymentService, compiler.NewEngine(), nil, nil, alertManager) - err := jobService.ReplaceAll(ctx, sampleTenant, incomingSpecs, jobNamesWithInvalidSpec, logWriter) + err = jobService.ReplaceAll(ctx, sampleTenant, incomingSpecs, jobNamesWithInvalidSpec, logWriter) assert.NoError(t, err) }) t.Run("should not return error if modified the job and the downstream in one time", func(t *testing.T) { @@ -2461,8 +2459,8 @@ func TestJobService(t *testing.T) { incomingSpecs := []*job.Spec{specA, specB} - w2, _ := models.NewWindow(jobVersion, "d", "0h", "24h") - existingJobWindow := window.NewCustomConfig(w2) + existingJobWindow, err := window.NewConfig("1d", "", "", "") + assert.NoError(t, err) existingSpecA, _ := job.NewSpecBuilder(jobVersion, "job-A", "sample-owner", jobSchedule, existingJobWindow, jobTask).WithAsset(jobAsset).Build() existingJobA := job.NewJob(sampleTenant, existingSpecA, jobADestination, jobAUpstreamName, false) existingSpecB, _ := job.NewSpecBuilder(jobVersion, "job-B", "sample-owner", jobSchedule, existingJobWindow, jobTask).WithAsset(jobAsset).Build() @@ -2506,7 +2504,7 @@ func TestJobService(t *testing.T) { }), jobNamesToRemove).Return(nil) jobService := service.NewJobService(jobRepo, upstreamRepo, downstreamRepo, pluginService, upstreamResolver, tenantDetailsGetter, eventHandler, log, jobDeploymentService, compiler.NewEngine(), nil, nil, alertManager) - err := jobService.ReplaceAll(ctx, sampleTenant, incomingSpecs, jobNamesWithInvalidSpec, logWriter) + err = jobService.ReplaceAll(ctx, sampleTenant, incomingSpecs, jobNamesWithInvalidSpec, logWriter) assert.NoError(t, err) }) t.Run("skips invalid job when classifying specs as added, modified, or deleted", func(t *testing.T) { @@ -2544,8 +2542,8 @@ func TestJobService(t *testing.T) { specB, _ := job.NewSpecBuilder(jobVersion, "job-B", "sample-owner", jobSchedule, jobWindow, jobTask).WithAsset(jobAsset).Build() incomingSpecs := []*job.Spec{specA, specB} - w2, _ := models.NewWindow(jobVersion, "d", "0h", "24h") - existingJobWindow := window.NewCustomConfig(w2) + existingJobWindow, err := window.NewConfig("1d", "", "", "") + assert.NoError(t, err) existingSpecB, _ := job.NewSpecBuilder(jobVersion, "job-B", "sample-owner", jobSchedule, existingJobWindow, jobTask).WithAsset(jobAsset).Build() existingJobB := job.NewJob(sampleTenant, existingSpecB, resource.ZeroURN(), nil, false) existingSpecC, _ := job.NewSpecBuilder(jobVersion, "job-C", "sample-owner", jobSchedule, jobWindow, jobTask).WithAsset(jobAsset).Build() @@ -2596,7 +2594,7 @@ func TestJobService(t *testing.T) { jobDeploymentService.On("UploadJobs", ctx, sampleTenant, emptyStringArr, jobNamesToRemove).Return(nil) jobService := service.NewJobService(jobRepo, upstreamRepo, downstreamRepo, pluginService, upstreamResolver, tenantDetailsGetter, eventHandler, log, jobDeploymentService, compiler.NewEngine(), nil, nil, alertManager) - err := jobService.ReplaceAll(ctx, sampleTenant, incomingSpecs, []job.Name{"job-D"}, logWriter) + err = jobService.ReplaceAll(ctx, sampleTenant, incomingSpecs, []job.Name{"job-D"}, logWriter) assert.NoError(t, err) }) t.Run("skips adding new invalid jobs, such jobs should be marked as dirty, and left like that, also dont process other jobs to be deleted and early return", func(t *testing.T) { @@ -2676,8 +2674,8 @@ func TestJobService(t *testing.T) { specB, _ := job.NewSpecBuilder(jobVersion, "job-B", "sample-owner", jobSchedule, jobWindow, jobTask).WithAsset(jobAsset).Build() incomingSpecs := []*job.Spec{specB} - w2, _ := models.NewWindow(jobVersion, "d", "0h", "24h") - existingJobWindow := window.NewCustomConfig(w2) + existingJobWindow, err := window.NewConfig("1d", "", "", "") + assert.NoError(t, err) existingSpecB, _ := job.NewSpecBuilder(jobVersion, "job-B", "sample-owner", jobSchedule, existingJobWindow, jobTask).WithAsset(jobAsset).Build() existingJobB := job.NewJob(sampleTenant, existingSpecB, resource.ZeroURN(), nil, false) existingSpecC, _ := job.NewSpecBuilder(jobVersion, "job-C", "sample-owner", jobSchedule, jobWindow, jobTask).WithAsset(jobAsset).Build() @@ -2692,7 +2690,7 @@ func TestJobService(t *testing.T) { alertManager := new(AlertManager) jobService := service.NewJobService(jobRepo, upstreamRepo, downstreamRepo, pluginService, upstreamResolver, tenantDetailsGetter, eventHandler, log, jobDeploymentService, compiler.NewEngine(), nil, nil, alertManager) - err := jobService.ReplaceAll(ctx, sampleTenant, incomingSpecs, jobNamesWithInvalidSpec, logWriter) + err = jobService.ReplaceAll(ctx, sampleTenant, incomingSpecs, jobNamesWithInvalidSpec, logWriter) assert.ErrorContains(t, err, "internal error") }) t.Run("skips to delete jobs if the downstream is not deleted", func(t *testing.T) { @@ -2883,8 +2881,8 @@ func TestJobService(t *testing.T) { incomingSpecs := []*job.Spec{specA} - w2, _ := models.NewWindow(jobVersion, "d", "0h", "24h") - existingJobWindow := window.NewCustomConfig(w2) + existingJobWindow, err := window.NewConfig("1d", "", "", "") + assert.NoError(t, err) existingSpecA, _ := job.NewSpecBuilder(jobVersion, "job-A", "sample-owner", jobSchedule, existingJobWindow, jobTask).WithAsset(jobAsset).Build() existingJobA := job.NewJob(sampleTenant, existingSpecA, jobADestination, jobAUpstreamName, false) existingSpecs := []*job.Job{existingJobA} @@ -2901,7 +2899,7 @@ func TestJobService(t *testing.T) { logWriter.On("Write", mock.Anything, mock.Anything).Return(nil) jobService := service.NewJobService(jobRepo, upstreamRepo, downstreamRepo, pluginService, upstreamResolver, tenantDetailsGetter, nil, log, jobDeploymentService, compiler.NewEngine(), nil, nil, nil) - err := jobService.ReplaceAll(ctx, sampleTenant, incomingSpecs, jobNamesWithInvalidSpec, logWriter) + err = jobService.ReplaceAll(ctx, sampleTenant, incomingSpecs, jobNamesWithInvalidSpec, logWriter) assert.ErrorContains(t, err, "internal error") }) t.Run("should not break process if one of job failed to be deleted", func(t *testing.T) { @@ -6189,8 +6187,8 @@ type JobRunInputCompiler struct { } // Compile provides a mock function with given fields: ctx, job, config, executedAt -func (_m *JobRunInputCompiler) Compile(ctx context.Context, job *scheduler.JobWithDetails, config scheduler.RunConfig, executedAt time.Time) (*scheduler.ExecutorInput, error) { - ret := _m.Called(ctx, job, config, executedAt) +func (_m *JobRunInputCompiler) Compile(ctx context.Context, job *scheduler.JobWithDetails, config scheduler.RunConfig) (*scheduler.ExecutorInput, error) { + ret := _m.Called(ctx, job, config) if len(ret) == 0 { panic("no return value specified for Compile") @@ -6198,19 +6196,19 @@ func (_m *JobRunInputCompiler) Compile(ctx context.Context, job *scheduler.JobWi var r0 *scheduler.ExecutorInput var r1 error - if rf, ok := ret.Get(0).(func(context.Context, *scheduler.JobWithDetails, scheduler.RunConfig, time.Time) (*scheduler.ExecutorInput, error)); ok { - return rf(ctx, job, config, executedAt) + if rf, ok := ret.Get(0).(func(context.Context, *scheduler.JobWithDetails, scheduler.RunConfig) (*scheduler.ExecutorInput, error)); ok { + return rf(ctx, job, config) } - if rf, ok := ret.Get(0).(func(context.Context, *scheduler.JobWithDetails, scheduler.RunConfig, time.Time) *scheduler.ExecutorInput); ok { - r0 = rf(ctx, job, config, executedAt) + if rf, ok := ret.Get(0).(func(context.Context, *scheduler.JobWithDetails, scheduler.RunConfig) *scheduler.ExecutorInput); ok { + r0 = rf(ctx, job, config) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*scheduler.ExecutorInput) } } - if rf, ok := ret.Get(1).(func(context.Context, *scheduler.JobWithDetails, scheduler.RunConfig, time.Time) error); ok { - r1 = rf(ctx, job, config, executedAt) + if rf, ok := ret.Get(1).(func(context.Context, *scheduler.JobWithDetails, scheduler.RunConfig) error); ok { + r1 = rf(ctx, job, config) } else { r1 = ret.Error(1) } diff --git a/core/job/spec_test.go b/core/job/spec_test.go index 7d2e9082b6..d2307f6432 100644 --- a/core/job/spec_test.go +++ b/core/job/spec_test.go @@ -9,7 +9,6 @@ import ( "github.com/goto/optimus/core/tenant" "github.com/goto/optimus/internal/lib/labels" "github.com/goto/optimus/internal/lib/window" - "github.com/goto/optimus/internal/models" ) func TestEntitySpec(t *testing.T) { @@ -24,8 +23,7 @@ func TestEntitySpec(t *testing.T) { WithRetry(retry). WithDependsOnPast(false). Build() - w, _ := models.NewWindow(jobVersion, "d", "24h", "24h") - jobWindow := window.NewCustomConfig(w) + w, _ := window.NewConfig("1d", "1d", "", "") jobTaskConfig, _ := job.ConfigFrom(map[string]string{"sample_task_key": "sample_value"}) jobTask := job.NewTask("bq2bq", jobTaskConfig, "") description := "sample description" @@ -50,7 +48,7 @@ func TestEntitySpec(t *testing.T) { t.Run("Spec", func(t *testing.T) { t.Run("should return values as inserted", func(t *testing.T) { - specA, err := job.NewSpecBuilder(jobVersion, "job-A", "sample-owner", jobSchedule, jobWindow, jobTask). + specA, err := job.NewSpecBuilder(jobVersion, "job-A", "sample-owner", jobSchedule, w, jobTask). WithDescription(description). WithLabels(lbl).WithHooks([]*job.Hook{hook}).WithAlerts([]*job.AlertSpec{alert}). WithSpecUpstream(specUpstream). @@ -75,8 +73,6 @@ func TestEntitySpec(t *testing.T) { assert.Equal(t, jobSchedule.CatchUp(), specA.Schedule().CatchUp()) assert.Equal(t, jobSchedule.Interval(), specA.Schedule().Interval()) - assert.Equal(t, jobWindow.Window, specA.WindowConfig().Window) - assert.Equal(t, jobTask, specA.Task()) assert.Equal(t, jobTask.Name(), specA.Task().Name()) assert.Equal(t, jobTask.Name().String(), specA.Task().Name().String()) @@ -124,7 +120,7 @@ func TestEntitySpec(t *testing.T) { "test_key": "", }) - actualSpec, actualError := job.NewSpecBuilder(jobVersion, "job-A", "sample-owner", jobSchedule, jobWindow, jobTask). + actualSpec, actualError := job.NewSpecBuilder(jobVersion, "job-A", "sample-owner", jobSchedule, w, jobTask). WithDescription(description). WithLabels(lbl).WithHooks([]*job.Hook{hook}).WithAlerts([]*job.AlertSpec{alert}). WithSpecUpstream(specUpstream). @@ -139,10 +135,10 @@ func TestEntitySpec(t *testing.T) { t.Run("Specs", func(t *testing.T) { t.Run("ToNameAndSpecMap should return map with name key and spec value", func(t *testing.T) { - specA, err := job.NewSpecBuilder(jobVersion, "job-A", "sample-owner", jobSchedule, jobWindow, jobTask).Build() + specA, err := job.NewSpecBuilder(jobVersion, "job-A", "sample-owner", jobSchedule, w, jobTask).Build() assert.NoError(t, err) - specB, err := job.NewSpecBuilder(jobVersion, "job-B", "sample-owner", jobSchedule, jobWindow, jobTask).Build() + specB, err := job.NewSpecBuilder(jobVersion, "job-B", "sample-owner", jobSchedule, w, jobTask).Build() assert.NoError(t, err) expectedMap := map[job.Name]*job.Spec{ @@ -157,10 +153,10 @@ func TestEntitySpec(t *testing.T) { }) t.Run("ToFullNameAndSpecMap should return map with fullname key and spec value", func(t *testing.T) { projectName := tenant.ProjectName("sample-project") - specA, err := job.NewSpecBuilder(jobVersion, "job-A", "sample-owner", jobSchedule, jobWindow, jobTask).Build() + specA, err := job.NewSpecBuilder(jobVersion, "job-A", "sample-owner", jobSchedule, w, jobTask).Build() assert.NoError(t, err) - specB, err := job.NewSpecBuilder(jobVersion, "job-B", "sample-owner", jobSchedule, jobWindow, jobTask).Build() + specB, err := job.NewSpecBuilder(jobVersion, "job-B", "sample-owner", jobSchedule, w, jobTask).Build() assert.NoError(t, err) expectedMap := map[job.FullName]*job.Spec{ @@ -175,13 +171,13 @@ func TestEntitySpec(t *testing.T) { }) t.Run("GetValid should return valid specs", func(t *testing.T) { - specA1, err := job.NewSpecBuilder(jobVersion, "job-A", "sample-owner", jobSchedule, jobWindow, jobTask).Build() + specA1, err := job.NewSpecBuilder(jobVersion, "job-A", "sample-owner", jobSchedule, w, jobTask).Build() assert.NoError(t, err) - specA2, err := job.NewSpecBuilder(jobVersion, "job-A", "sample-owner", jobSchedule, jobWindow, jobTask).Build() + specA2, err := job.NewSpecBuilder(jobVersion, "job-A", "sample-owner", jobSchedule, w, jobTask).Build() assert.NoError(t, err) - specB, err := job.NewSpecBuilder(jobVersion, "job-B", "sample-owner", jobSchedule, jobWindow, jobTask).Build() + specB, err := job.NewSpecBuilder(jobVersion, "job-B", "sample-owner", jobSchedule, w, jobTask).Build() assert.NoError(t, err) expectedValidSpecs := []*job.Spec{specB} diff --git a/core/scheduler/service/executor_input_compiler.go b/core/scheduler/service/executor_input_compiler.go index e8f8e7e3b9..6855f0ee32 100644 --- a/core/scheduler/service/executor_input_compiler.go +++ b/core/scheduler/service/executor_input_compiler.go @@ -92,7 +92,7 @@ func getJobLabelsString(labels map[string]string) string { return strings.Join(labelStringArray, ",") } -func (i InputCompiler) Compile(ctx context.Context, job *scheduler.JobWithDetails, config scheduler.RunConfig, executedAt time.Time) (*scheduler.ExecutorInput, error) { +func (i InputCompiler) Compile(ctx context.Context, job *scheduler.JobWithDetails, config scheduler.RunConfig) (*scheduler.ExecutorInput, error) { tenantDetails, err := i.tenantService.GetDetails(ctx, job.Job.Tenant) if err != nil { i.logger.Error("error getting tenant details: %s", err) @@ -109,7 +109,7 @@ func (i InputCompiler) Compile(ctx context.Context, job *scheduler.JobWithDetail return nil, err } - systemDefinedVars := getSystemDefinedConfigs(job.Job, interval, executedAt, config.ScheduledAt) + systemDefinedVars := getSystemDefinedConfigs(job.Job, interval, config.ScheduledAt) // Prepare template context and compile task config taskContext := compiler.PrepareContext( @@ -218,20 +218,16 @@ func (i InputCompiler) compileConfigs(configs map[string]string, templateCtx map return conf, secretsConfig, nil } -func getSystemDefinedConfigs(job *scheduler.Job, interval interval.Interval, executedAt, scheduledAt time.Time) map[string]string { +func getSystemDefinedConfigs(job *scheduler.Job, interval interval.Interval, scheduledAt time.Time) map[string]string { vars := map[string]string{ configDstart: interval.Start().Format(TimeISOFormat), configDend: interval.End().Format(TimeISOFormat), - configExecutionTime: executedAt.Format(TimeSQLFormat), // TODO: remove this once ali support RFC3339 format configDestination: job.Destination.String(), - } - // TODO: remove this condition after v1/v2 removal, add to map directly - if job.WindowConfig.GetVersion() == window.NewWindowVersion { - vars[configStartDate] = interval.Start().Format(time.DateOnly) - vars[configEndDate] = interval.End().Format(time.DateOnly) - vars[configExecutionTime] = scheduledAt.Format(TimeSQLFormat) // TODO: remove this once ali support RFC3339 format - vars[configScheduleTime] = scheduledAt.Format(TimeISOFormat) - vars[configScheduleDate] = scheduledAt.Format(time.DateOnly) + configStartDate: interval.Start().Format(time.DateOnly), + configEndDate: interval.End().Format(time.DateOnly), + configExecutionTime: scheduledAt.Format(TimeSQLFormat), // TODO: remove this once ali support RFC3339 format + configScheduleTime: scheduledAt.Format(TimeISOFormat), + configScheduleDate: scheduledAt.Format(time.DateOnly), } return vars diff --git a/core/scheduler/service/executor_input_compiler_test.go b/core/scheduler/service/executor_input_compiler_test.go index 747e4117f0..8f9ca1a12d 100644 --- a/core/scheduler/service/executor_input_compiler_test.go +++ b/core/scheduler/service/executor_input_compiler_test.go @@ -19,7 +19,6 @@ import ( "github.com/goto/optimus/core/tenant" "github.com/goto/optimus/internal/lib/interval" "github.com/goto/optimus/internal/lib/window" - "github.com/goto/optimus/internal/models" ) func TestExecutorCompiler(t *testing.T) { @@ -68,19 +67,24 @@ func TestExecutorCompiler(t *testing.T) { defer tenantService.AssertExpectations(t) inputCompiler := service.NewJobInputCompiler(tenantService, nil, nil, logger) - inputExecutor, err := inputCompiler.Compile(ctx, &details, config, currentTime.Add(time.Hour)) + inputExecutor, err := inputCompiler.Compile(ctx, &details, config) assert.NotNil(t, err) assert.EqualError(t, err, "get details error") assert.Nil(t, inputExecutor) }) t.Run("should give error if get interval fails", func(t *testing.T) { - w1, _ := models.NewWindow(1, "d", "2h", "2") - window1 := window.NewCustomConfig(w1) + conf := window.SimpleConfig{ + Size: "1d", + ShiftBy: "", + Location: "", + TruncateTo: "2", + } + window2 := window.NewCustomConfigWithTimezone(conf) job := scheduler.Job{ Name: "job1", Tenant: tnnt, - WindowConfig: window1, + WindowConfig: window2, } details := scheduler.JobWithDetails{ Job: &job, @@ -103,15 +107,14 @@ func TestExecutorCompiler(t *testing.T) { defer tenantService.AssertExpectations(t) inputCompiler := service.NewJobInputCompiler(tenantService, nil, nil, logger) - inputExecutor, err := inputCompiler.Compile(ctx, &details, config, currentTime.Add(time.Hour)) + inputExecutor, err := inputCompiler.Compile(ctx, &details, config) assert.NotNil(t, err) - assert.EqualError(t, err, "failed to parse task window with size 2: time: missing unit in duration \"2\"") + assert.ErrorContains(t, err, "invalid value for unit 2, accepted values are [h,d,w,M,y]") assert.Nil(t, inputExecutor) }) t.Run("should give error if CompileJobRunAssets fails", func(t *testing.T) { - w, _ := models.NewWindow(2, "d", "1h", "24h") - cw := window.NewCustomConfig(w) + cw, _ := window.NewConfig("1d", "1h", "", "") job := scheduler.Job{ Name: "job1", Tenant: tnnt, @@ -141,14 +144,19 @@ func TestExecutorCompiler(t *testing.T) { tenantService.On("GetDetails", ctx, tnnt).Return(tenantDetails, nil) defer tenantService.AssertExpectations(t) - interval, err := window.FromBaseWindow(w).GetInterval(config.ScheduledAt) + win, err := window.FromCustomConfig(cw.GetSimpleConfig()) + assert.NoError(t, err) + intr, err := win.GetInterval(config.ScheduledAt) assert.NoError(t, err) - executedAt := currentTime.Add(time.Hour) systemDefinedVars := map[string]string{ - "DSTART": interval.Start().Format(time.RFC3339), - "DEND": interval.End().Format(time.RFC3339), - "EXECUTION_TIME": executedAt.Format(time.DateTime), + "DSTART": intr.Start().Format(time.RFC3339), + "DEND": intr.End().Format(time.RFC3339), + "EXECUTION_TIME": config.ScheduledAt.Format(time.DateTime), "JOB_DESTINATION": job.Destination.String(), + "END_DATE": intr.End().Format(time.DateOnly), + "SCHEDULE_DATE": config.ScheduledAt.Format(time.DateOnly), + "SCHEDULE_TIME": config.ScheduledAt.Format(service.TimeISOFormat), + "START_DATE": intr.Start().Format(time.DateOnly), } taskContext := mock.Anything @@ -156,19 +164,19 @@ func TestExecutorCompiler(t *testing.T) { templateCompiler.On("Compile", mock.Anything, taskContext).Return(map[string]string{}, nil) defer templateCompiler.AssertExpectations(t) assetCompiler := new(mockAssetCompiler) - assetCompiler.On("CompileJobRunAssets", ctx, &job, systemDefinedVars, interval, taskContext).Return(nil, fmt.Errorf("CompileJobRunAssets error")) + assetCompiler.On("CompileJobRunAssets", ctx, &job, systemDefinedVars, intr, taskContext).Return(nil, fmt.Errorf("CompileJobRunAssets error")) defer assetCompiler.AssertExpectations(t) inputCompiler := service.NewJobInputCompiler(tenantService, templateCompiler, assetCompiler, logger) - inputExecutor, err := inputCompiler.Compile(ctx, &details, config, executedAt) + inputExecutor, err := inputCompiler.Compile(ctx, &details, config) assert.NotNil(t, err) assert.EqualError(t, err, "CompileJobRunAssets error") assert.Nil(t, inputExecutor) }) t.Run("compileConfigs for Executor type Task ", func(t *testing.T) { - w1, _ := models.NewWindow(2, "d", "1h", "24h") - window1 := window.NewCustomConfig(w1) + window1, err := window.NewConfig("1d", "1d", "", "") + assert.NoError(t, err) job := scheduler.Job{ Name: "job1", Tenant: tnnt, @@ -209,15 +217,20 @@ func TestExecutorCompiler(t *testing.T) { tenantService.On("GetDetails", ctx, tnnt).Return(tenantDetails, nil) defer tenantService.AssertExpectations(t) - interval, err := window.FromBaseWindow(w1).GetInterval(config.ScheduledAt) + win, err := window.FromCustomConfig(window1.GetSimpleConfig()) + assert.NoError(t, err) + intr, err := win.GetInterval(config.ScheduledAt) assert.NoError(t, err) - executedAt := currentTime.Add(time.Hour) systemDefinedVars := map[string]string{ - "DSTART": interval.Start().Format(time.RFC3339), - "DEND": interval.End().Format(time.RFC3339), - "EXECUTION_TIME": executedAt.Format(time.DateTime), + "DSTART": intr.Start().Format(time.RFC3339), + "DEND": intr.End().Format(time.RFC3339), + "EXECUTION_TIME": config.ScheduledAt.Format(time.DateTime), "JOB_DESTINATION": job.Destination.String(), + "END_DATE": intr.End().Format(time.DateOnly), + "SCHEDULE_DATE": config.ScheduledAt.Format(time.DateOnly), + "SCHEDULE_TIME": config.ScheduledAt.Format(service.TimeISOFormat), + "START_DATE": intr.Start().Format(time.DateOnly), } taskContext := mock.Anything @@ -233,7 +246,7 @@ func TestExecutorCompiler(t *testing.T) { assetCompiler := new(mockAssetCompiler) defer assetCompiler.AssertExpectations(t) inputCompiler := service.NewJobInputCompiler(tenantService, templateCompiler, assetCompiler, logger) - inputExecutor, err := inputCompiler.Compile(ctx, &details, config, executedAt) + inputExecutor, err := inputCompiler.Compile(ctx, &details, config) assert.NotNil(t, err) assert.EqualError(t, err, "some.config compilation error") @@ -249,7 +262,7 @@ func TestExecutorCompiler(t *testing.T) { assetCompiler := new(mockAssetCompiler) defer assetCompiler.AssertExpectations(t) inputCompiler := service.NewJobInputCompiler(tenantService, templateCompiler, assetCompiler, logger) - inputExecutor, err := inputCompiler.Compile(ctx, &details, config, executedAt) + inputExecutor, err := inputCompiler.Compile(ctx, &details, config) assert.NotNil(t, err) assert.EqualError(t, err, "secret.config compilation error") @@ -263,19 +276,23 @@ func TestExecutorCompiler(t *testing.T) { Return(map[string]string{"secret.config.compiled": "a.secret.val.compiled"}, nil) defer templateCompiler.AssertExpectations(t) assetCompiler := new(mockAssetCompiler) - assetCompiler.On("CompileJobRunAssets", ctx, &job, systemDefinedVars, interval, taskContext).Return(compiledFile, nil) + assetCompiler.On("CompileJobRunAssets", ctx, &job, systemDefinedVars, intr, taskContext).Return(compiledFile, nil) defer assetCompiler.AssertExpectations(t) inputCompiler := service.NewJobInputCompiler(tenantService, templateCompiler, assetCompiler, logger) - inputExecutorResp, err := inputCompiler.Compile(ctx, &details, config, executedAt) + inputExecutorResp, err := inputCompiler.Compile(ctx, &details, config) assert.Nil(t, err) expectedInputExecutor := &scheduler.ExecutorInput{ Configs: map[string]string{ - "DSTART": interval.Start().Format(time.RFC3339), - "DEND": interval.End().Format(time.RFC3339), - "EXECUTION_TIME": executedAt.Format(time.DateTime), + "DSTART": intr.Start().Format(time.RFC3339), + "DEND": intr.End().Format(time.RFC3339), "JOB_DESTINATION": job.Destination.String(), "some.config.compiled": "val.compiled", + "EXECUTION_TIME": config.ScheduledAt.Format(time.DateTime), + "END_DATE": intr.End().Format(time.DateOnly), + "SCHEDULE_DATE": config.ScheduledAt.Format(time.DateOnly), + "SCHEDULE_TIME": config.ScheduledAt.Format(service.TimeISOFormat), + "START_DATE": intr.Start().Format(time.DateOnly), }, Secrets: map[string]string{"secret.config.compiled": "a.secret.val.compiled"}, Files: compiledFile, @@ -313,21 +330,25 @@ func TestExecutorCompiler(t *testing.T) { } assetCompilerNew := new(mockAssetCompiler) - assetCompilerNew.On("CompileJobRunAssets", ctx, &jobNew, systemDefinedVars, interval, taskContext).Return(compiledFile, nil) + assetCompilerNew.On("CompileJobRunAssets", ctx, &jobNew, systemDefinedVars, intr, taskContext).Return(compiledFile, nil) defer assetCompilerNew.AssertExpectations(t) inputCompiler := service.NewJobInputCompiler(tenantService, templateCompiler, assetCompilerNew, logger) - inputExecutorResp, err := inputCompiler.Compile(ctx, &detailsNew, config, executedAt) + inputExecutorResp, err := inputCompiler.Compile(ctx, &detailsNew, config) assert.Nil(t, err) expectedInputExecutor := &scheduler.ExecutorInput{ Configs: map[string]string{ - "DSTART": interval.Start().Format(time.RFC3339), - "DEND": interval.End().Format(time.RFC3339), - "EXECUTION_TIME": executedAt.Format(time.DateTime), + "DSTART": intr.Start().Format(time.RFC3339), + "DEND": intr.End().Format(time.RFC3339), "JOB_DESTINATION": job.Destination.String(), "some.config.compiled": "val.compiled", + "EXECUTION_TIME": config.ScheduledAt.Format(time.DateTime), + "END_DATE": intr.End().Format(time.DateOnly), + "SCHEDULE_DATE": config.ScheduledAt.Format(time.DateOnly), + "SCHEDULE_TIME": config.ScheduledAt.Format(service.TimeISOFormat), + "START_DATE": intr.Start().Format(time.DateOnly), }, Secrets: map[string]string{"secret.config.compiled": "a.secret.val.compiled"}, Files: compiledFile, @@ -349,8 +370,8 @@ func TestExecutorCompiler(t *testing.T) { }) }) t.Run("compileConfigs for Executor type Hook", func(t *testing.T) { - w1, _ := models.NewWindow(2, "d", "1h", "24h") - window1 := window.NewCustomConfig(w1) + window1, err := window.NewConfig("1d", "1h", "", "") + assert.NoError(t, err) job := scheduler.Job{ Name: "job1", Tenant: tnnt, @@ -393,14 +414,19 @@ func TestExecutorCompiler(t *testing.T) { tenantService.On("GetDetails", ctx, tnnt).Return(tenantDetails, nil) defer tenantService.AssertExpectations(t) - interval, err := window.FromBaseWindow(w1).GetInterval(config.ScheduledAt) + win, err := window.FromCustomConfig(window1.GetSimpleConfig()) + assert.NoError(t, err) + intr, err := win.GetInterval(config.ScheduledAt) assert.NoError(t, err) - executedAt := currentTime.Add(time.Hour) systemDefinedVars := map[string]string{ - "DSTART": interval.Start().Format(time.RFC3339), - "DEND": interval.End().Format(time.RFC3339), - "EXECUTION_TIME": executedAt.Format(time.DateTime), + "DSTART": intr.Start().Format(time.RFC3339), + "DEND": intr.End().Format(time.RFC3339), "JOB_DESTINATION": job.Destination.String(), + "END_DATE": intr.End().Format(time.DateOnly), + "EXECUTION_TIME": config.ScheduledAt.Format(time.DateTime), + "SCHEDULE_DATE": config.ScheduledAt.Format(time.DateOnly), + "SCHEDULE_TIME": config.ScheduledAt.Format(service.TimeISOFormat), + "START_DATE": intr.Start().Format(time.DateOnly), } taskContext := mock.Anything @@ -408,7 +434,7 @@ func TestExecutorCompiler(t *testing.T) { "someFileName": "fileContents", } assetCompiler := new(mockAssetCompiler) - assetCompiler.On("CompileJobRunAssets", ctx, &job, systemDefinedVars, interval, taskContext).Return(compiledFile, nil) + assetCompiler.On("CompileJobRunAssets", ctx, &job, systemDefinedVars, intr, taskContext).Return(compiledFile, nil) defer assetCompiler.AssertExpectations(t) templateCompiler := new(mockTemplateCompiler) @@ -423,17 +449,21 @@ func TestExecutorCompiler(t *testing.T) { defer templateCompiler.AssertExpectations(t) inputCompiler := service.NewJobInputCompiler(tenantService, templateCompiler, assetCompiler, logger) - inputExecutorResp, err := inputCompiler.Compile(ctx, &details, config, executedAt) + inputExecutorResp, err := inputCompiler.Compile(ctx, &details, config) assert.Nil(t, err) expectedInputExecutor := &scheduler.ExecutorInput{ Configs: map[string]string{ - "DSTART": interval.Start().Format(time.RFC3339), - "DEND": interval.End().Format(time.RFC3339), - "EXECUTION_TIME": executedAt.Format(time.DateTime), + "DSTART": intr.Start().Format(time.RFC3339), + "DEND": intr.End().Format(time.RFC3339), + "EXECUTION_TIME": config.ScheduledAt.Format(time.DateTime), "JOB_DESTINATION": job.Destination.String(), "hook.compiled": "hook.val.compiled", "JOB_LABELS": "job_id=00000000-0000-0000-0000-000000000000,job_name=job1,namespace=ns1,project=proj1", + "END_DATE": intr.End().Format(time.DateOnly), + "SCHEDULE_DATE": config.ScheduledAt.Format(time.DateOnly), + "SCHEDULE_TIME": config.ScheduledAt.Format(service.TimeISOFormat), + "START_DATE": intr.Start().Format(time.DateOnly), }, Secrets: map[string]string{"secret.hook.compiled": "hook.s.val.compiled"}, Files: compiledFile, @@ -448,8 +478,7 @@ func TestExecutorCompiler(t *testing.T) { assert.Equal(t, expectedInputExecutor, inputExecutorResp) }) t.Run("compileConfigs for Executor type Hook should fail if error in hook compilation", func(t *testing.T) { - w1, _ := models.NewWindow(2, "d", "1h", "24h") - window1 := window.NewCustomConfig(w1) + window1, _ := window.NewConfig("1d", "1h", "", "") job := scheduler.Job{ Name: "job1", Tenant: tnnt, @@ -492,14 +521,19 @@ func TestExecutorCompiler(t *testing.T) { tenantService.On("GetDetails", ctx, tnnt).Return(tenantDetails, nil) defer tenantService.AssertExpectations(t) - interval, err := window.FromBaseWindow(w1).GetInterval(config.ScheduledAt) + win, err := window.FromCustomConfig(window1.GetSimpleConfig()) + assert.NoError(t, err) + intr, err := win.GetInterval(config.ScheduledAt) assert.NoError(t, err) - executedAt := currentTime.Add(time.Hour) systemDefinedVars := map[string]string{ - "DSTART": interval.Start().Format(time.RFC3339), - "DEND": interval.End().Format(time.RFC3339), - "EXECUTION_TIME": executedAt.Format(time.DateTime), + "DSTART": intr.Start().Format(time.RFC3339), + "DEND": intr.End().Format(time.RFC3339), + "EXECUTION_TIME": config.ScheduledAt.Format(time.DateTime), "JOB_DESTINATION": job.Destination.String(), + "END_DATE": intr.End().Format(time.DateOnly), + "SCHEDULE_DATE": config.ScheduledAt.Format(time.DateOnly), + "SCHEDULE_TIME": config.ScheduledAt.Format(service.TimeISOFormat), + "START_DATE": intr.Start().Format(time.DateOnly), } taskContext := mock.Anything @@ -507,7 +541,7 @@ func TestExecutorCompiler(t *testing.T) { "someFileName": "fileContents", } assetCompiler := new(mockAssetCompiler) - assetCompiler.On("CompileJobRunAssets", ctx, &job, systemDefinedVars, interval, taskContext).Return(compiledFile, nil) + assetCompiler.On("CompileJobRunAssets", ctx, &job, systemDefinedVars, intr, taskContext).Return(compiledFile, nil) defer assetCompiler.AssertExpectations(t) templateCompiler := new(mockTemplateCompiler) @@ -521,15 +555,14 @@ func TestExecutorCompiler(t *testing.T) { defer templateCompiler.AssertExpectations(t) inputCompiler := service.NewJobInputCompiler(tenantService, templateCompiler, assetCompiler, logger) - inputExecutorResp, err := inputCompiler.Compile(ctx, &details, config, executedAt) + inputExecutorResp, err := inputCompiler.Compile(ctx, &details, config) assert.NotNil(t, err) assert.EqualError(t, err, "error in compiling hook template") assert.Nil(t, inputExecutorResp) }) t.Run("compileConfigs for Executor type Hook, should raise error if hooks not there in job", func(t *testing.T) { - w1, _ := models.NewWindow(2, "d", "1h", "24h") - window1 := window.NewCustomConfig(w1) + window1, _ := window.NewConfig("1d", "1h", "", "") job := scheduler.Job{ Name: "job1", Tenant: tnnt, @@ -564,14 +597,19 @@ func TestExecutorCompiler(t *testing.T) { tenantService.On("GetDetails", ctx, tnnt).Return(tenantDetails, nil) defer tenantService.AssertExpectations(t) - interval, err := window.FromBaseWindow(w1).GetInterval(config.ScheduledAt) + win, err := window.FromCustomConfig(window1.GetSimpleConfig()) + assert.NoError(t, err) + intr, err := win.GetInterval(config.ScheduledAt) assert.NoError(t, err) - executedAt := currentTime.Add(time.Hour) systemDefinedVars := map[string]string{ - "DSTART": interval.Start().Format(time.RFC3339), - "DEND": interval.End().Format(time.RFC3339), - "EXECUTION_TIME": executedAt.Format(time.DateTime), + "DSTART": intr.Start().Format(time.RFC3339), + "DEND": intr.End().Format(time.RFC3339), + "EXECUTION_TIME": config.ScheduledAt.Format(time.DateTime), "JOB_DESTINATION": job.Destination.String(), + "END_DATE": intr.End().Format(time.DateOnly), + "SCHEDULE_DATE": config.ScheduledAt.Format(time.DateOnly), + "SCHEDULE_TIME": config.ScheduledAt.Format(service.TimeISOFormat), + "START_DATE": intr.Start().Format(time.DateOnly), } taskContext := mock.Anything @@ -579,7 +617,7 @@ func TestExecutorCompiler(t *testing.T) { "someFileName": "fileContents", } assetCompiler := new(mockAssetCompiler) - assetCompiler.On("CompileJobRunAssets", ctx, &job, systemDefinedVars, interval, taskContext).Return(compiledFile, nil) + assetCompiler.On("CompileJobRunAssets", ctx, &job, systemDefinedVars, intr, taskContext).Return(compiledFile, nil) defer assetCompiler.AssertExpectations(t) templateCompiler := new(mockTemplateCompiler) @@ -590,7 +628,7 @@ func TestExecutorCompiler(t *testing.T) { defer templateCompiler.AssertExpectations(t) inputCompiler := service.NewJobInputCompiler(tenantService, templateCompiler, assetCompiler, logger) - inputExecutorResp, err := inputCompiler.Compile(ctx, &details, config, executedAt) + inputExecutorResp, err := inputCompiler.Compile(ctx, &details, config) assert.NotNil(t, err) assert.Nil(t, inputExecutorResp) diff --git a/core/scheduler/service/job_run_asset_compiler_test.go b/core/scheduler/service/job_run_asset_compiler_test.go index 090fd00a57..dacc79d38b 100644 --- a/core/scheduler/service/job_run_asset_compiler_test.go +++ b/core/scheduler/service/job_run_asset_compiler_test.go @@ -15,7 +15,6 @@ import ( "github.com/goto/optimus/core/tenant" "github.com/goto/optimus/internal/compiler" "github.com/goto/optimus/internal/lib/window" - "github.com/goto/optimus/internal/models" ) func TestJobAssetsCompiler(t *testing.T) { @@ -28,8 +27,7 @@ func TestJobAssetsCompiler(t *testing.T) { tnnt, _ := tenant.NewTenant(project.Name().String(), namespace.Name().String()) currentTime := time.Now() scheduleTime := currentTime.Add(-time.Hour) - w1, _ := models.NewWindow(2, "d", "1h", "24h") - windowConfig1 := window.NewCustomConfig(w1) + w1, _ := window.NewConfig("1d", "1h", "", "") job := &scheduler.Job{ Name: "jobName", @@ -42,14 +40,15 @@ func TestJobAssetsCompiler(t *testing.T) { }, }, Hooks: nil, - WindowConfig: windowConfig1, + WindowConfig: w1, Assets: map[string]string{ "assetName": "assetValue", "query.sql": "select 1;", }, } - w2 := window.FromBaseWindow(w1) - interval, err := w2.GetInterval(scheduleTime) + win, err := window.FromCustomConfig(w1.GetSimpleConfig()) + assert.NoError(t, err) + interval, err := win.GetInterval(scheduleTime) assert.NoError(t, err) executedAt := currentTime.Add(time.Hour) diff --git a/core/scheduler/service/job_run_service.go b/core/scheduler/service/job_run_service.go index e0d5b865bf..abfb8704da 100644 --- a/core/scheduler/service/job_run_service.go +++ b/core/scheduler/service/job_run_service.go @@ -23,7 +23,6 @@ import ( "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" "github.com/goto/optimus/internal/utils/filter" ) @@ -61,7 +60,6 @@ type JobRepository interface { } type JobRunRepository interface { - GetByID(ctx context.Context, id scheduler.JobRunID) (*scheduler.JobRun, error) GetByScheduledAt(ctx context.Context, tenant tenant.Tenant, name scheduler.JobName, scheduledAt time.Time) (*scheduler.JobRun, error) GetLatestRun(ctx context.Context, project tenant.ProjectName, name scheduler.JobName, status *scheduler.State) (*scheduler.JobRun, error) GetRunsByTimeRange(ctx context.Context, project tenant.ProjectName, jobName scheduler.JobName, status *scheduler.State, since, until time.Time) ([]*scheduler.JobRun, error) @@ -85,7 +83,7 @@ type OperatorRunRepository interface { } type JobInputCompiler interface { - Compile(ctx context.Context, job *scheduler.JobWithDetails, config scheduler.RunConfig, executedAt time.Time) (*scheduler.ExecutorInput, error) + Compile(ctx context.Context, job *scheduler.JobWithDetails, config scheduler.RunConfig) (*scheduler.ExecutorInput, error) } type PriorityResolver interface { @@ -131,25 +129,7 @@ func (s *JobRunService) JobRunInput(ctx context.Context, projectName tenant.Proj s.l.Error("error getting job [%s]: %s", jobName, err) return nil, err } - // TODO: Use scheduled_at instead of executed_at for computations, for deterministic calculations - // Todo: later, always return scheduleTime, for scheduleTimes greater than a given date - var jobRun *scheduler.JobRun - if config.JobRunID.IsEmpty() { - s.l.Warn("getting job run by scheduled at") - jobRun, err = s.repo.GetByScheduledAt(ctx, details.Job.Tenant, jobName, config.ScheduledAt) - } else { - s.l.Warn("getting job run by id") - jobRun, err = s.repo.GetByID(ctx, config.JobRunID) - } - var executedAt time.Time - if err != nil { // Fallback for executed_at to scheduled_at - executedAt = config.ScheduledAt - s.l.Warn("suppressed error is encountered when getting job run: %s", err) - } else { - executedAt = jobRun.StartTime - } - // Additional task config from existing replay replayJobConfig, err := s.replayRepo.GetReplayJobConfig(ctx, details.Job.Tenant, details.Job.Name, config.ScheduledAt) if err != nil { s.l.Error("error getting replay job config from db: %s", err) @@ -159,7 +139,7 @@ func (s *JobRunService) JobRunInput(ctx context.Context, projectName tenant.Proj details.Job.Task.Config[k] = v } - return s.compiler.Compile(ctx, details, config, executedAt) + return s.compiler.Compile(ctx, details, config) } func (s *JobRunService) GetJobRunsByFilter(ctx context.Context, projectName tenant.ProjectName, jobName scheduler.JobName, filters ...filter.FilterOpt) ([]*scheduler.JobRun, error) { @@ -462,52 +442,27 @@ func (s *JobRunService) getInterval(project *tenant.Project, job *scheduler.JobW return w.GetInterval(referenceTime) } - 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() - } - - config.ShiftBy = "" - config.TruncateTo = string(duration.None) - - cw, err := window.FromCustomConfig(config) + 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 } - return cw.GetInterval(referenceTime) - } - w, err := models.NewWindow(windowConfig.GetVersion(), "", "0", windowConfig.GetSize()) - if err != nil { - s.l.Error("error initializing window: %v", err) - return interval.Interval{}, err - } - - if err := w.Validate(); err != nil { - s.l.Error("error validating window: %v", err) - return interval.Interval{}, err + config = preset.Config() + } else { + config = windowConfig.GetSimpleConfig() } - startTime, err := w.GetStartTime(referenceTime) - if err != nil { - return interval.Interval{}, err - } + config.ShiftBy = "" + config.TruncateTo = string(duration.None) - endTime, err := w.GetEndTime(referenceTime) + cw, err := window.FromCustomConfig(config) if err != nil { return interval.Interval{}, err } - - return interval.NewInterval(startTime, endTime), nil + return cw.GetInterval(referenceTime) } func getExpectedRuns(spec *cron.ScheduleSpec, startTime, endTime time.Time) []*scheduler.JobRunStatus { diff --git a/core/scheduler/service/job_run_service_test.go b/core/scheduler/service/job_run_service_test.go index 76486959e5..bcee03e333 100644 --- a/core/scheduler/service/job_run_service_test.go +++ b/core/scheduler/service/job_run_service_test.go @@ -776,128 +776,6 @@ func TestJobRunService(t *testing.T) { assert.NotNil(t, err) assert.EqualError(t, err, "some error") }) - t.Run("should get jobRunByScheduledAt if job run id is not given", func(t *testing.T) { - tnnt, _ := tenant.NewTenant(projName.String(), namespaceName.String()) - job := scheduler.Job{ - Name: jobName, - Tenant: tnnt, - Task: &scheduler.Task{ - Config: map[string]string{}, - }, - } - details := scheduler.JobWithDetails{Job: &job} - - someScheduleTime := todayDate.Add(time.Hour * 24 * -1) - executedAt := todayDate.Add(time.Hour * 23 * -1) - startTime := executedAt - runConfig := scheduler.RunConfig{ - Executor: scheduler.Executor{}, - ScheduledAt: someScheduleTime, - JobRunID: scheduler.JobRunID{}, - } - - jobRepo := new(JobRepository) - jobRepo.On("GetJobDetails", ctx, projName, jobName). - Return(&details, nil) - defer jobRepo.AssertExpectations(t) - - jobRun := scheduler.JobRun{ - JobName: jobName, - Tenant: tnnt, - StartTime: startTime, - } - jobRunRepo := new(mockJobRunRepository) - jobRunRepo.On("GetByScheduledAt", ctx, tnnt, jobName, someScheduleTime). - Return(&jobRun, nil) - defer jobRunRepo.AssertExpectations(t) - - dummyExecutorInput := scheduler.ExecutorInput{ - Configs: scheduler.ConfigMap{ - "someKey": "someValue", - }, - } - jobToCompile := job - jobToCompile.Task.Config["EXECUTION_PROJECT"] = "example" - jobToCompileDetails := scheduler.JobWithDetails{Job: &jobToCompile} - - jobReplayRepo := new(ReplayRepository) - jobReplayRepo.On("GetReplayJobConfig", ctx, tnnt, jobName, someScheduleTime).Return(map[string]string{"EXECUTION_PROJECT": "example"}, nil) - defer jobReplayRepo.AssertExpectations(t) - - jobInputCompiler := new(mockJobInputCompiler) - jobInputCompiler.On("Compile", ctx, &jobToCompileDetails, runConfig, executedAt). - Return(&dummyExecutorInput, nil) - defer jobInputCompiler.AssertExpectations(t) - - runService := service.NewJobRunService(logger, - jobRepo, jobRunRepo, jobReplayRepo, nil, nil, nil, jobInputCompiler, nil, nil, feats) - executorInput, err := runService.JobRunInput(ctx, projName, jobName, runConfig) - - assert.Equal(t, &dummyExecutorInput, executorInput) - assert.Nil(t, err) - }) - t.Run("should use GetByID if job run id is given", func(t *testing.T) { - tnnt, _ := tenant.NewTenant(projName.String(), namespaceName.String()) - job := scheduler.Job{ - Name: jobName, - Tenant: tnnt, - Task: &scheduler.Task{ - Config: map[string]string{}, - }, - } - details := scheduler.JobWithDetails{Job: &job} - - someScheduleTime := todayDate.Add(time.Hour * 24 * -1) - executedAt := todayDate.Add(time.Hour * 23 * -1) - startTime := executedAt - jobRunID := scheduler.JobRunID(uuid.New()) - runConfig := scheduler.RunConfig{ - Executor: scheduler.Executor{}, - ScheduledAt: someScheduleTime, - JobRunID: jobRunID, - } - - jobRepo := new(JobRepository) - jobRepo.On("GetJobDetails", ctx, projName, jobName). - Return(&details, nil) - defer jobRepo.AssertExpectations(t) - - jobRun := scheduler.JobRun{ - JobName: jobName, - Tenant: tnnt, - StartTime: startTime, - } - jobRunRepo := new(mockJobRunRepository) - jobRunRepo.On("GetByID", ctx, jobRunID). - Return(&jobRun, nil) - defer jobRunRepo.AssertExpectations(t) - - dummyExecutorInput := scheduler.ExecutorInput{ - Configs: scheduler.ConfigMap{ - "someKey": "someValue", - }, - } - - jobToCompile := job - jobToCompile.Task.Config["EXECUTION_PROJECT"] = "example" - jobToCompileDetails := scheduler.JobWithDetails{Job: &jobToCompile} - - jobReplayRepo := new(ReplayRepository) - jobReplayRepo.On("GetReplayJobConfig", ctx, tnnt, jobName, someScheduleTime).Return(map[string]string{"EXECUTION_PROJECT": "example"}, nil) - defer jobReplayRepo.AssertExpectations(t) - - jobInputCompiler := new(mockJobInputCompiler) - jobInputCompiler.On("Compile", ctx, &jobToCompileDetails, runConfig, executedAt). - Return(&dummyExecutorInput, nil) - defer jobInputCompiler.AssertExpectations(t) - - runService := service.NewJobRunService(logger, - jobRepo, jobRunRepo, jobReplayRepo, nil, nil, nil, jobInputCompiler, nil, nil, feats) - executorInput, err := runService.JobRunInput(ctx, projName, jobName, runConfig) - - assert.Equal(t, &dummyExecutorInput, executorInput) - assert.Nil(t, err) - }) t.Run("should handle if job run is not found , and fallback to execution time being schedule time", func(t *testing.T) { tnnt, _ := tenant.NewTenant(projName.String(), namespaceName.String()) job := scheduler.Job{ @@ -922,11 +800,6 @@ func TestJobRunService(t *testing.T) { Return(&details, nil) defer jobRepo.AssertExpectations(t) - jobRunRepo := new(mockJobRunRepository) - jobRunRepo.On("GetByID", ctx, jobRunID). - Return(&scheduler.JobRun{}, errors.NotFound(scheduler.EntityJobRun, "no record for job:"+jobName.String())) - defer jobRunRepo.AssertExpectations(t) - dummyExecutorInput := scheduler.ExecutorInput{ Configs: scheduler.ConfigMap{ "someKey": "someValue", @@ -942,71 +815,17 @@ func TestJobRunService(t *testing.T) { defer jobReplayRepo.AssertExpectations(t) jobInputCompiler := new(mockJobInputCompiler) - jobInputCompiler.On("Compile", ctx, &jobToCompileDetails, runConfig, someScheduleTime). + jobInputCompiler.On("Compile", ctx, &jobToCompileDetails, runConfig). Return(&dummyExecutorInput, nil) defer jobInputCompiler.AssertExpectations(t) runService := service.NewJobRunService(logger, - jobRepo, jobRunRepo, jobReplayRepo, nil, nil, nil, jobInputCompiler, nil, nil, feats) + jobRepo, nil, jobReplayRepo, nil, nil, nil, jobInputCompiler, nil, nil, feats) executorInput, err := runService.JobRunInput(ctx, projName, jobName, runConfig) assert.Equal(t, &dummyExecutorInput, executorInput) assert.Nil(t, err) }) - t.Run("should not return error if get job run fails", func(t *testing.T) { - tnnt, _ := tenant.NewTenant(projName.String(), namespaceName.String()) - job := scheduler.Job{ - Name: jobName, - Tenant: tnnt, - Task: &scheduler.Task{ - Config: map[string]string{}, - }, - } - details := scheduler.JobWithDetails{Job: &job} - - someScheduleTime := todayDate.Add(time.Hour * 24 * -1) - jobRunID := scheduler.JobRunID(uuid.New()) - runConfig := scheduler.RunConfig{ - Executor: scheduler.Executor{}, - ScheduledAt: someScheduleTime, - JobRunID: jobRunID, - } - - jobRepo := new(JobRepository) - jobRepo.On("GetJobDetails", ctx, projName, jobName). - Return(&details, nil) - defer jobRepo.AssertExpectations(t) - - jobRunRepo := new(mockJobRunRepository) - jobRunRepo.On("GetByID", ctx, jobRunID). - Return(&scheduler.JobRun{}, fmt.Errorf("some error other than not found error ")) - defer jobRunRepo.AssertExpectations(t) - - dummyExecutorInput := scheduler.ExecutorInput{ - Configs: scheduler.ConfigMap{ - "someKey": "someValue", - }, - } - jobToCompile := job - jobToCompile.Task.Config["EXECUTION_PROJECT"] = "example" - jobToCompileDetails := scheduler.JobWithDetails{Job: &jobToCompile} - - jobReplayRepo := new(ReplayRepository) - jobReplayRepo.On("GetReplayJobConfig", ctx, tnnt, jobName, someScheduleTime).Return(map[string]string{"EXECUTION_PROJECT": "example"}, nil) - defer jobReplayRepo.AssertExpectations(t) - - jobInputCompiler := new(mockJobInputCompiler) - jobInputCompiler.On("Compile", ctx, &jobToCompileDetails, runConfig, someScheduleTime). - Return(&dummyExecutorInput, nil) - defer jobInputCompiler.AssertExpectations(t) - - runService := service.NewJobRunService(logger, - jobRepo, jobRunRepo, jobReplayRepo, nil, nil, nil, jobInputCompiler, nil, nil, feats) - executorInput, err := runService.JobRunInput(ctx, projName, jobName, runConfig) - - assert.Nil(t, err) - assert.Equal(t, &dummyExecutorInput, executorInput) - }) }) t.Run("GetJobRunList", func(t *testing.T) { @@ -1896,8 +1715,8 @@ type mockJobInputCompiler struct { mock.Mock } -func (m *mockJobInputCompiler) Compile(ctx context.Context, job *scheduler.JobWithDetails, config scheduler.RunConfig, executedAt time.Time) (*scheduler.ExecutorInput, error) { - args := m.Called(ctx, job, config, executedAt) +func (m *mockJobInputCompiler) Compile(ctx context.Context, job *scheduler.JobWithDetails, config scheduler.RunConfig) (*scheduler.ExecutorInput, error) { + args := m.Called(ctx, job, config) if args.Get(0) == nil { return nil, args.Error(1) } @@ -1908,14 +1727,6 @@ type mockJobRunRepository struct { mock.Mock } -func (m *mockJobRunRepository) GetByID(ctx context.Context, id scheduler.JobRunID) (*scheduler.JobRun, error) { - args := m.Called(ctx, id) - if args.Get(0) == nil { - return nil, args.Error(1) - } - return args.Get(0).(*scheduler.JobRun), args.Error(1) -} - func (m *mockJobRunRepository) GetRunsByInterval(ctx context.Context, project tenant.ProjectName, jobName scheduler.JobName, interval interval.Interval) ([]*scheduler.JobRun, error) { args := m.Called(ctx, project, jobName, interval) if args.Get(0) == nil { diff --git a/ext/scheduler/airflow/dag/compiler_test.go b/ext/scheduler/airflow/dag/compiler_test.go index 6faef6a1c4..cbf045e368 100644 --- a/ext/scheduler/airflow/dag/compiler_test.go +++ b/ext/scheduler/airflow/dag/compiler_test.go @@ -14,7 +14,6 @@ import ( "github.com/goto/optimus/ext/scheduler/airflow/dag" "github.com/goto/optimus/internal/errors" "github.com/goto/optimus/internal/lib/window" - "github.com/goto/optimus/internal/models" "github.com/goto/optimus/plugin" ) @@ -135,8 +134,7 @@ func setProject(tnnt tenant.Tenant, airflowVersion string) *tenant.Project { } func setupJobDetails(tnnt tenant.Tenant) *scheduler.JobWithDetails { - w1, err := models.NewWindow(1, "d", "0", "1h") - window1 := window.NewCustomConfig(w1) + window1, err := window.NewConfig("1h", "", "", "d") if err != nil { panic(err) } diff --git a/internal/lib/window/config.go b/internal/lib/window/config.go index beb1fd9f30..7bd426935e 100644 --- a/internal/lib/window/config.go +++ b/internal/lib/window/config.go @@ -6,7 +6,6 @@ import ( "github.com/goto/optimus/internal/errors" "github.com/goto/optimus/internal/lib/duration" - "github.com/goto/optimus/internal/models" ) const NewWindowVersion = 3 @@ -30,46 +29,9 @@ type Config struct { windowType Type Preset string - // kept for backward compatibility, will be removed later - Window models.Window - simple SimpleConfig } -// Following functions are for backward compatibility - -func (c Config) GetSize() string { // nolint: gocritic - if c.Window == nil { - return c.simple.Size - } - - return c.Window.GetSize() -} - -func (c Config) GetOffset() string { // nolint: gocritic - if c.Window == nil { - return c.simple.ShiftBy - } - - return c.Window.GetOffset() -} - -func (c Config) GetTruncateTo() string { // nolint: gocritic - if c.Window == nil { - return c.simple.TruncateTo - } - - return c.Window.GetTruncateTo() -} - -func (c Config) GetVersion() int { - if c.Window == nil { - return NewWindowVersion - } - - return c.Window.GetVersion() -} - func (c Config) GetSimpleConfig() SimpleConfig { return c.simple } @@ -86,13 +48,6 @@ func NewPresetConfig(preset string) (Config, error) { }, nil } -func NewCustomConfig(w models.Window) Config { - return Config{ - windowType: Custom, - Window: w, - } -} - func NewSimpleConfig(size, shiftBy, location, truncateTo string) (SimpleConfig, error) { validationErr := errors.NewMultiError("error in window config") diff --git a/internal/lib/window/config_test.go b/internal/lib/window/config_test.go index 0dc801519f..5be7544288 100644 --- a/internal/lib/window/config_test.go +++ b/internal/lib/window/config_test.go @@ -6,7 +6,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/goto/optimus/internal/lib/window" - "github.com/goto/optimus/internal/models" ) func TestWindowConfig(t *testing.T) { @@ -20,34 +19,19 @@ func TestWindowConfig(t *testing.T) { config, err := window.NewPresetConfig(preset) assert.NoError(t, err) assert.Equal(t, "yesterday", config.Preset) - assert.Equal(t, "", config.GetSize()) - assert.Equal(t, "", config.GetOffset()) - assert.Equal(t, "", config.GetTruncateTo()) - assert.Equal(t, 3, config.GetVersion()) assert.Equal(t, "preset", string(config.Type())) }) }) t.Run("Custom Window config", func(t *testing.T) { - t.Run("creates a custom window config", func(t *testing.T) { - w, err := models.NewWindow(2, "d", "0", "24h") - assert.NoError(t, err) - config := window.NewCustomConfig(w) - assert.Equal(t, "", config.Preset) - assert.Equal(t, "24h", config.GetSize()) - assert.Equal(t, "0", config.GetOffset()) - assert.Equal(t, "d", config.GetTruncateTo()) - assert.Equal(t, 2, config.GetVersion()) - assert.Equal(t, "custom", string(config.Type())) - }) t.Run("creates a custom window config version 3", func(t *testing.T) { config, err := window.NewConfig("1d", "-1h", "", "") assert.NoError(t, err) assert.Equal(t, "", config.Preset) - assert.Equal(t, "1d", config.GetSize()) - assert.Equal(t, "-1h", config.GetOffset()) - assert.Equal(t, "", config.GetTruncateTo()) - assert.Equal(t, 3, config.GetVersion()) + sc := config.GetSimpleConfig() + assert.Equal(t, "1d", sc.Size) + assert.Equal(t, "-1h", sc.ShiftBy) + assert.Equal(t, "", sc.TruncateTo) assert.Equal(t, "custom", string(config.Type())) }) }) @@ -55,10 +39,6 @@ func TestWindowConfig(t *testing.T) { t.Run("creates a incremental window config", func(t *testing.T) { config := window.NewIncrementalConfig() assert.Equal(t, "", config.Preset) - assert.Equal(t, "", config.GetSize()) - assert.Equal(t, "", config.GetOffset()) - assert.Equal(t, "", config.GetTruncateTo()) - assert.Equal(t, 3, config.GetVersion()) assert.Equal(t, "incremental", string(config.Type())) }) }) diff --git a/internal/lib/window/factory.go b/internal/lib/window/factory.go index 261f195b8f..3c9ecb101b 100644 --- a/internal/lib/window/factory.go +++ b/internal/lib/window/factory.go @@ -27,9 +27,5 @@ func From[T WithConfig](config Config, schedule string, getter func(string) (T, return FromCustomConfig(preset.Config()) } - if config.Window == nil { - return FromCustomConfig(config.GetSimpleConfig()) - } - - return FromBaseWindow(config.Window), nil + return FromCustomConfig(config.GetSimpleConfig()) } diff --git a/internal/lib/window/factory_test.go b/internal/lib/window/factory_test.go index 1ecdc8293b..bb8b0f0a80 100644 --- a/internal/lib/window/factory_test.go +++ b/internal/lib/window/factory_test.go @@ -8,7 +8,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/goto/optimus/internal/lib/window" - "github.com/goto/optimus/internal/models" ) type Preset struct { @@ -37,7 +36,7 @@ func TestWindowFactory(t *testing.T) { config, err := window.NewPresetConfig("yesterday") assert.NoError(t, err) - _, err = window.From[Preset](config, "", func(_ string) (Preset, error) { + _, err = window.From(config, "", func(_ string) (Preset, error) { return Preset{}, errors.New("cannot get window") }) assert.Error(t, err) @@ -46,7 +45,7 @@ func TestWindowFactory(t *testing.T) { config, err := window.NewPresetConfig("yesterday") assert.NoError(t, err) - w, err := window.From[Preset](config, "", func(_ string) (Preset, error) { + w, err := window.From(config, "", func(_ string) (Preset, error) { conf := window.SimpleConfig{ Size: "1d", ShiftBy: "", @@ -70,7 +69,7 @@ func TestWindowFactory(t *testing.T) { config, err := window.NewConfig("1d", "", "", "") assert.NoError(t, err) - w, err := window.From[Preset](config, "", func(_ string) (Preset, error) { + w, err := window.From(config, "", func(_ string) (Preset, error) { return Preset{ Name: "yesterday", config: config.GetSimpleConfig(), @@ -78,19 +77,6 @@ func TestWindowFactory(t *testing.T) { }) assert.NoError(t, err) - sept1 := time.Date(2023, 9, 1, 1, 0, 0, 0, time.UTC) - interval, err := w.GetInterval(sept1) - assert.NoError(t, err) - assert.Equal(t, "2023-08-31T00:00:00Z", interval.Start().Format(time.RFC3339)) - assert.Equal(t, "2023-09-01T00:00:00Z", interval.End().Format(time.RFC3339)) - }) - t.Run("creates window from base window", func(t *testing.T) { - w1, err := models.NewWindow(2, "d", "0", "24h") - assert.NoError(t, err) - - w, err := window.From[Preset](window.NewCustomConfig(w1), "", nil) - assert.NoError(t, err) - sept1 := time.Date(2023, 9, 1, 1, 0, 0, 0, time.UTC) interval, err := w.GetInterval(sept1) assert.NoError(t, err) diff --git a/internal/lib/window/old_window.go b/internal/lib/window/old_window.go deleted file mode 100644 index 2f4b10f74e..0000000000 --- a/internal/lib/window/old_window.go +++ /dev/null @@ -1,30 +0,0 @@ -package window - -import ( - "time" - - "github.com/goto/optimus/internal/lib/interval" - "github.com/goto/optimus/internal/models" -) - -type OldWindow struct { - window models.Window -} - -func (w OldWindow) GetInterval(referenceTime time.Time) (interval.Interval, error) { - endTime, err := w.window.GetEndTime(referenceTime) - if err != nil { - return interval.Interval{}, err - } - - startTime, err := w.window.GetStartTime(referenceTime) - if err != nil { - return interval.Interval{}, err - } - - return interval.NewInterval(startTime, endTime), nil -} - -func FromBaseWindow(w models.Window) OldWindow { - return OldWindow{window: w} -} diff --git a/internal/lib/window/old_window_test.go b/internal/lib/window/old_window_test.go deleted file mode 100644 index 81a0cb8df8..0000000000 --- a/internal/lib/window/old_window_test.go +++ /dev/null @@ -1,61 +0,0 @@ -package window_test - -import ( - "testing" - "time" - - "github.com/stretchr/testify/assert" - - "github.com/goto/optimus/internal/lib/window" - "github.com/goto/optimus/internal/models" -) - -func TestOldWindow(t *testing.T) { - t.Run("FromBaseWindow", func(t *testing.T) { - t.Run("returns error when unable to get end of window", func(t *testing.T) { - w1, _ := models.NewWindow(2, "Z", "0", "24h") - baseWindow := window.FromBaseWindow(w1) - - sept1 := time.Date(2023, 9, 1, 1, 0, 0, 0, time.UTC) - _, err := baseWindow.GetInterval(sept1) - assert.Error(t, err) - assert.ErrorContains(t, err, "error validating truncate_to") - }) - t.Run("returns error when unable to get start of window", func(t *testing.T) { - w1, _ := models.NewWindow(2, "M", "0", "bM") - baseWindow := window.FromBaseWindow(w1) - - sept1 := time.Date(2023, 9, 1, 1, 0, 0, 0, time.UTC) - _, err := baseWindow.GetInterval(sept1) - assert.Error(t, err) - assert.ErrorContains(t, err, "error validating size") - }) - t.Run("returns the interval for window", func(t *testing.T) { - w1, _ := models.NewWindow(2, "d", "0", "24h") - baseWindow := window.FromBaseWindow(w1) - - sept1 := time.Date(2023, 9, 1, 1, 0, 0, 0, time.UTC) - interval, err := baseWindow.GetInterval(sept1) - assert.NoError(t, err) - assert.Equal(t, "2023-08-31T00:00:00Z", interval.Start().Format(time.RFC3339)) - assert.Equal(t, "2023-09-01T00:00:00Z", interval.End().Format(time.RFC3339)) - }) - t.Run("should return zero and error if error parsing schedule", func(t *testing.T) { - schedule := "-1 * * * *" - - actualWindow, actualError := window.FromSchedule(schedule) - - assert.Zero(t, actualWindow) - assert.Error(t, actualError) - assert.ErrorContains(t, actualError, "unable to parse job cron interval") - }) - t.Run("should return window and nil if no error is encountered", func(t *testing.T) { - schedule := "0 * * * *" - - actualWindow, actualError := window.FromSchedule(schedule) - - assert.NotZero(t, actualWindow) - assert.NoError(t, actualError) - }) - }) -} diff --git a/internal/models/window.go b/internal/models/window.go deleted file mode 100644 index d7a928c9ef..0000000000 --- a/internal/models/window.go +++ /dev/null @@ -1,64 +0,0 @@ -package models - -import ( - "fmt" - "strconv" - "strings" - "time" -) - -type Window interface { - Validate() error - - GetStartTime(scheduleTime time.Time) (time.Time, error) - GetEndTime(scheduleTime time.Time) (time.Time, error) - GetTruncateTo() string - GetOffset() string - GetSize() string - GetVersion() int -} - -func NewWindow(version int, truncateTo, offset, size string) (Window, error) { - if version == 1 { - return windowV1{truncateTo: truncateTo, offset: offset, size: size}, nil - } - if version == 2 { // nolint:mnd - return windowV2{truncateTo: truncateTo, offset: offset, size: size}, nil - } - return nil, fmt.Errorf("window version [%d] is not recognized", version) -} - -// GetEndRunDate subtract 1 day to make end inclusive -func GetEndRunDate(runTime time.Time, window Window) (time.Time, error) { - months, nonMonthDurationString, err := monthsAndNonMonthExpression(window.GetSize()) - if err != nil { - return time.Time{}, err - } - - nonMonthDuration, err := time.ParseDuration(nonMonthDurationString) - if err != nil { - return time.Time{}, err - } - return runTime.Add(nonMonthDuration).AddDate(0, months, 0).Add(time.Hour * -24), nil -} - -func monthsAndNonMonthExpression(durationExpression string) (int, string, error) { - if !strings.Contains(durationExpression, "M") { - return 0, durationExpression, nil - } - maxSubString := 2 - splits := strings.SplitN(durationExpression, "M", maxSubString) - months, err := strconv.Atoi(splits[0]) - if err != nil { - return 0, "0", err - } - // duration contains only month - if len(splits) == 1 || splits[1] == "" { - return months, "0", nil - } - // if duration is negative then use the negative duration for both the splits. - if months < 0 { - return months, "-" + splits[1], nil - } - return months, splits[1], nil -} diff --git a/internal/models/window_test.go b/internal/models/window_test.go deleted file mode 100644 index f6da0b2353..0000000000 --- a/internal/models/window_test.go +++ /dev/null @@ -1,44 +0,0 @@ -package models_test - -import ( - "testing" - - "github.com/stretchr/testify/assert" - - "github.com/goto/optimus/internal/models" -) - -func TestNewWindow(t *testing.T) { - t.Run("should return window and nil if version is recognized", func(t *testing.T) { - testCases := []struct { - version int - truncateTo string - offset string - size string - - testMessage string - }{ - {version: 1, truncateTo: "h", offset: "1h", size: "1h", testMessage: "version 1"}, - {version: 2, truncateTo: "h", offset: "1h", size: "1h", testMessage: "version 2"}, - } - - for _, tCase := range testCases { - actualWindow, actualError := models.NewWindow(tCase.version, tCase.truncateTo, tCase.offset, tCase.size) - - assert.NotNil(t, actualWindow, tCase.testMessage) - assert.NoError(t, actualError, tCase.testMessage) - } - }) - - t.Run("should return nil and error if version is not recognized", func(t *testing.T) { - version := 0 - truncateTo := "h" - offset := "1h" - size := "1h" - - actualWindow, actualError := models.NewWindow(version, truncateTo, offset, size) - - assert.Nil(t, actualWindow) - assert.Error(t, actualError) - }) -} diff --git a/internal/models/window_v1.go b/internal/models/window_v1.go deleted file mode 100644 index 57a83787ab..0000000000 --- a/internal/models/window_v1.go +++ /dev/null @@ -1,204 +0,0 @@ -package models - -import ( - "errors" - "fmt" - "regexp" - "strconv" - "strings" - "time" -) - -const ( - HoursInDay = 24 - HoursInMonth = HoursInDay * 30 -) - -var monthExp = regexp.MustCompile("(\\+|-)?([0-9]+)(M)") // nolint - -type windowV1 struct { - truncateTo string - offset string - size string -} - -func (w windowV1) Validate() error { - _, err := w.prepareWindow() // nolint:dogsled - return err -} - -func (windowV1) GetVersion() int { - return 1 -} - -func (w windowV1) GetTruncateTo() string { - return w.truncateTo -} - -func (w windowV1) GetOffset() string { - if w.offset != "" { - return w.offset - } - return w.inHrs(0) -} - -func (w windowV1) GetSize() string { - if w.size != "" { - return w.size - } - return w.inHrs(HoursInDay) -} - -func (w windowV1) GetStartTime(scheduledAt time.Time) (startTime time.Time, err error) { - jobSpecTaskWindow, err := w.prepareWindow() - if err != nil { - return - } - startTime, _ = jobSpecTaskWindow.getWindowDate(scheduledAt, jobSpecTaskWindow.Size, jobSpecTaskWindow.Offset, jobSpecTaskWindow.TruncateTo) - return -} - -func (w windowV1) GetEndTime(scheduledAt time.Time) (endTime time.Time, err error) { - jobSpecTaskWindow, err := w.prepareWindow() - if err != nil { - return - } - _, endTime = jobSpecTaskWindow.getWindowDate(scheduledAt, jobSpecTaskWindow.Size, jobSpecTaskWindow.Offset, jobSpecTaskWindow.TruncateTo) - return -} - -type JobSpecTaskWindow struct { - Size time.Duration - Offset time.Duration - TruncateTo string -} - -func (w *windowV1) prepareWindow() (JobSpecTaskWindow, error) { - var err error - window := JobSpecTaskWindow{} - window.Size = time.Hour * HoursInDay - window.Offset = 0 - window.TruncateTo = "" - - if w.truncateTo != "" { - window.TruncateTo = w.truncateTo - } - - // check if string contains monthly notation - if w.size != "" { - window.Size, err = w.tryParsingInMonths(w.size) - if err != nil { - // treat as normal duration - window.Size, err = time.ParseDuration(w.size) - if err != nil { - return window, fmt.Errorf("failed to parse task window with size %v: %w", w.size, err) - } - } - } - - // check if string contains monthly notation - if w.offset != "" { - window.Offset, err = w.tryParsingInMonths(w.offset) - if err != nil { - // treat as normal duration - window.Offset, err = time.ParseDuration(w.offset) - if err != nil { - return window, fmt.Errorf("failed to parse task window with offset %v: %w", w.offset, err) - } - } - } - - return window, nil -} - -func (*JobSpecTaskWindow) getWindowDate(today time.Time, windowSize, windowOffset time.Duration, windowTruncateTo string) (time.Time, time.Time) { - floatingEnd := today - - // apply truncation to end - switch windowTruncateTo { - case "h": - // remove time upto hours - floatingEnd = floatingEnd.Truncate(time.Hour) - case "d": - // remove time upto day - floatingEnd = floatingEnd.Truncate(time.Hour * HoursInDay) - case "w": - // shift current window to nearest Sunday - nearestSunday := time.Duration(time.Saturday-floatingEnd.Weekday()+1) * time.Hour * HoursInDay - floatingEnd = floatingEnd.Add(nearestSunday) - floatingEnd = floatingEnd.Truncate(time.Hour * HoursInDay) - } - - windowEnd := floatingEnd.Add(windowOffset) - windowStart := windowEnd.Add(-windowSize) - - // handle monthly windows separately as every month is not of same size - if windowTruncateTo == "M" { - floatingEnd = today - // shift current window to nearest month start and end - - // truncate the date - floatingEnd = time.Date(floatingEnd.Year(), floatingEnd.Month(), 1, 0, 0, 0, 0, time.UTC) - - // then add the month offset - // for handling offset, treat 30 days as 1 month - offsetMonths := windowOffset / (time.Hour * HoursInMonth) - floatingEnd = floatingEnd.AddDate(0, int(offsetMonths), 0) - - // then find the last day of this month - floatingEnd = floatingEnd.AddDate(0, 1, -1) - - // final end is computed - windowEnd = floatingEnd.Truncate(time.Hour * HoursInDay) - - // truncate days/hours from window start as well - floatingStart := time.Date(floatingEnd.Year(), floatingEnd.Month(), 1, 0, 0, 0, 0, time.UTC) - // for handling size, treat 30 days as 1 month, and as we have already truncated current month - // subtract 1 from this - sizeMonths := (windowSize / (time.Hour * HoursInMonth)) - 1 - if sizeMonths > 0 { - floatingStart = floatingStart.AddDate(0, int(-sizeMonths), 0) - } - - // final start is computed - windowStart = floatingStart - } - - return windowStart, windowEnd -} - -func (windowV1) inHrs(hrs int) string { - if hrs == 0 { - return "0" - } - return fmt.Sprintf("%dh", hrs) -} - -// check if string contains monthly notation -func (windowV1) tryParsingInMonths(str string) (time.Duration, error) { - sz := time.Duration(0) - monthMatches := monthExp.FindAllStringSubmatch(str, -1) - if len(monthMatches) > 0 && len(monthMatches[0]) == 4 { - // replace month notation with days first, treating 1M as 30 days - monthsCount, err := strconv.Atoi(monthMatches[0][2]) - if err != nil { - return sz, fmt.Errorf("failed to parse task configuration of %s: %w", str, err) - } - sz = time.Hour * HoursInMonth * time.Duration(monthsCount) - if monthMatches[0][1] == "-" { - sz *= -1 - } - - str = strings.TrimSpace(monthExp.ReplaceAllString(str, "")) - if len(str) > 0 { - // check if there is remaining time that we can still parse - remainingTime, err := time.ParseDuration(str) - if err != nil { - return sz, fmt.Errorf("failed to parse task configuration of %s: %w", str, err) - } - sz += remainingTime - } - return sz, nil - } - return sz, errors.New("invalid month string") -} diff --git a/internal/models/window_v1_test.go b/internal/models/window_v1_test.go deleted file mode 100644 index ff79f27aea..0000000000 --- a/internal/models/window_v1_test.go +++ /dev/null @@ -1,182 +0,0 @@ -package models_test - -import ( - "fmt" - "testing" - "time" - - "github.com/stretchr/testify/assert" - - "github.com/goto/optimus/internal/models" -) - -func TestWindowV1(t *testing.T) { - t.Run("Validate", func(t *testing.T) { - t.Run("should not return error for window size which is not a positive or an instant time duration", func(t *testing.T) { - validWindowConfigs := []string{"24h", "2h45m", "60s", "45m24h", "", "0"} - for _, config := range validWindowConfigs { - window, err := models.NewWindow(1, "", "", config) - if err != nil { - panic(err) - } - err = window.Validate() - assert.Nil(t, err, fmt.Sprintf("failed for : %s", config)) - } - }) - t.Run("should not return error for window offset which is a valid time duration", func(t *testing.T) { - validOffsetConfigs := []string{"24h", "2h45m", "60s", "45m24h", "0", ""} - for _, config := range validOffsetConfigs { - window, err := models.NewWindow(1, "", config, "") - if err != nil { - panic(err) - } - err = window.Validate() - assert.Nil(t, err, fmt.Sprintf("failed for : %s", config)) - } - }) - t.Run("should not return error for valid window truncate configs", func(t *testing.T) { - validTruncateConfigs := []string{"h", "d", "w", "M", ""} - for _, config := range validTruncateConfigs { - window, err := models.NewWindow(1, config, "", "") - if err != nil { - panic(err) - } - err = window.Validate() - assert.Nil(t, err, fmt.Sprintf("failed for : %s", config)) - } - }) - }) - t.Run("GetTimeRange", func(t *testing.T) { - t.Run("should provide start and end time window for the given schedule", func(t *testing.T) { - cases := []struct { - ScheduleTime time.Time - Size string - Offset string - TruncateTo string - ExpectedStartTime time.Time - ExpectedEndTime time.Time - }{ - { - ScheduleTime: time.Date(2021, 2, 25, 0, 0, 0, 0, time.UTC), - Size: "24h", - Offset: "0", - TruncateTo: "", - ExpectedStartTime: time.Date(2021, 2, 24, 0, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2021, 2, 25, 0, 0, 0, 0, time.UTC), - }, - { - ScheduleTime: time.Date(2020, 7, 10, 6, 33, 22, 0, time.UTC), - Size: "24h", - Offset: "0", - TruncateTo: "", - ExpectedStartTime: time.Date(2020, 7, 9, 6, 33, 22, 0, time.UTC), // modified from the original, since it was not consistent with the implementation default truncate. original [time.Date(2020, 7, 9, 6, 33, 22, 0, time.UTC)] - ExpectedEndTime: time.Date(2020, 7, 10, 6, 33, 22, 0, time.UTC), // modified from the original, since it was not consistent with the implementation default truncate. original [time.Date(2020, 7, 10, 6, 33, 22, 0, time.UTC)] - }, - { - ScheduleTime: time.Date(2020, 7, 10, 6, 33, 22, 0, time.UTC), - Size: "24h", - Offset: "0", - TruncateTo: "h", - ExpectedStartTime: time.Date(2020, 7, 9, 6, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2020, 7, 10, 6, 0, 0, 0, time.UTC), - }, - { - ScheduleTime: time.Date(2020, 7, 10, 6, 33, 22, 0, time.UTC), - Size: "24h", - Offset: "0", - TruncateTo: "d", - ExpectedStartTime: time.Date(2020, 7, 9, 0, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2020, 7, 10, 0, 0, 0, 0, time.UTC), - }, - { - ScheduleTime: time.Date(2020, 7, 10, 6, 33, 22, 0, time.UTC), - Size: "48h", - Offset: "24h", - TruncateTo: "d", - ExpectedStartTime: time.Date(2020, 7, 9, 0, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2020, 7, 11, 0, 0, 0, 0, time.UTC), - }, - { - ScheduleTime: time.Date(2020, 7, 10, 6, 33, 22, 0, time.UTC), - Size: "24h", - Offset: "-24h", - TruncateTo: "d", - ExpectedStartTime: time.Date(2020, 7, 8, 0, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2020, 7, 9, 0, 0, 0, 0, time.UTC), - }, - { - ScheduleTime: time.Date(2020, 7, 10, 6, 33, 22, 0, time.UTC), - Size: "168h", - Offset: "0", - TruncateTo: "w", - ExpectedStartTime: time.Date(2020, 7, 5, 0, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2020, 7, 12, 0, 0, 0, 0, time.UTC), - }, - { - ScheduleTime: time.Date(2021, 2, 25, 6, 33, 22, 0, time.UTC), - Size: "720h", - Offset: "0", - TruncateTo: "M", - ExpectedStartTime: time.Date(2021, 2, 1, 0, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2021, 2, 28, 0, 0, 0, 0, time.UTC), - }, - { - ScheduleTime: time.Date(2021, 2, 25, 6, 33, 22, 0, time.UTC), - Size: "720h", - Offset: "720h", - TruncateTo: "M", - ExpectedStartTime: time.Date(2021, 3, 1, 0, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2021, 3, 31, 0, 0, 0, 0, time.UTC), - }, - { - ScheduleTime: time.Date(2021, 2, 25, 6, 33, 22, 0, time.UTC), - Size: "720h", - Offset: "-720h", - TruncateTo: "M", - ExpectedStartTime: time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2021, 1, 31, 0, 0, 0, 0, time.UTC), - }, - { - ScheduleTime: time.Date(2021, 2, 25, 6, 33, 22, 0, time.UTC), - Size: "480h", - Offset: "0", - TruncateTo: "M", - ExpectedStartTime: time.Date(2021, 2, 1, 0, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2021, 2, 28, 0, 0, 0, 0, time.UTC), - }, - { - ScheduleTime: time.Date(2021, 2, 25, 6, 33, 22, 0, time.UTC), - Size: "1440h", - Offset: "0", - TruncateTo: "M", - ExpectedStartTime: time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2021, 2, 28, 0, 0, 0, 0, time.UTC), - }, - { - ScheduleTime: time.Date(2021, 3, 31, 6, 33, 22, 0, time.UTC), - Size: "720h", - Offset: "-720h", - TruncateTo: "M", - ExpectedStartTime: time.Date(2021, 2, 1, 0, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2021, 2, 28, 0, 0, 0, 0, time.UTC), - }, - } - for _, sc := range cases { - w, err := models.NewWindow(1, sc.TruncateTo, sc.Offset, sc.Size) - if err != nil { - panic(err) - } - - actualValidateError := w.Validate() - actualStartTime, actualStartTimeError := w.GetStartTime(sc.ScheduleTime) - actualEndTime, actualEndTimeError := w.GetEndTime(sc.ScheduleTime) - - assert.NoError(t, actualValidateError) - assert.Equal(t, sc.ExpectedStartTime.String(), actualStartTime.String()) - assert.NoError(t, actualStartTimeError) - assert.Equal(t, sc.ExpectedEndTime.String(), actualEndTime.String()) - assert.NoError(t, actualEndTimeError) - } - }) - }) -} diff --git a/internal/models/window_v2.go b/internal/models/window_v2.go deleted file mode 100644 index b0f34e5e49..0000000000 --- a/internal/models/window_v2.go +++ /dev/null @@ -1,176 +0,0 @@ -package models - -import ( - "errors" - "fmt" - "strings" - "time" - - "github.com/goto/optimus/internal/utils" -) - -type windowV2 struct { - truncateTo string - offset string - size string -} - -func (windowV2) GetVersion() int { - return 2 //nolint:mnd -} - -func (w windowV2) Validate() error { - if err := w.validateTruncateTo(); err != nil { - return fmt.Errorf("error validating truncate_to: %w", err) - } - if err := w.validateOffset(); err != nil { - return fmt.Errorf("error validating offset: %w", err) - } - if err := w.validateSize(); err != nil { - return fmt.Errorf("error validating size: %w", err) - } - return nil -} - -func (w windowV2) GetStartTime(scheduleTime time.Time) (time.Time, error) { - endTime, err := w.GetEndTime(scheduleTime) - if err != nil { - return time.Time{}, err - } - return w.getStartTime(endTime) -} - -func (w windowV2) GetEndTime(scheduleTime time.Time) (time.Time, error) { - if err := w.Validate(); err != nil { - return time.Time{}, err - } - truncatedTime := w.truncateTime(scheduleTime) - return w.adjustOffset(truncatedTime) -} - -func (w windowV2) GetTruncateTo() string { - return w.truncateTo -} - -func (w windowV2) GetOffset() string { - return w.offset -} - -func (w windowV2) GetSize() string { - return w.size -} - -func (w windowV2) validateTruncateTo() error { - if w.truncateTo == "" { - return nil - } - - validTruncateOptions := []string{"h", "d", "w", "M"} - // TODO: perhaps we can avoid using util, in hope we can remove this package - if !utils.ContainsString(validTruncateOptions, w.truncateTo) { - return fmt.Errorf("invalid option provided, provide one of: %v", validTruncateOptions) - } - return nil -} - -func (w windowV2) validateOffset() error { - if w.offset == "" { - return nil - } - - _, nonMonthDuration, err := monthsAndNonMonthExpression(w.offset) - if err != nil { - return err - } - if nonMonthDuration != "" { - if _, err := time.ParseDuration(nonMonthDuration); err != nil { - return fmt.Errorf("failed to parse task window with offset %v: %w", w.offset, err) - } - } - return nil -} - -func (w windowV2) validateSize() error { - if w.size == "" { - return nil - } - - months, nonMonthDuration, err := monthsAndNonMonthExpression(w.size) - if err != nil { - return err - } - if months < 0 { - return errors.New("size cannot be negative") - } - if nonMonthDuration != "" { - if _, err := time.ParseDuration(nonMonthDuration); err != nil { - return fmt.Errorf("failed to parse task window with size %v: %w", w.size, err) - } - } - if strings.HasPrefix(w.size, "-") { - return errors.New("size cannot be negative") - } - return nil -} - -func (w windowV2) truncateTime(scheduleTime time.Time) time.Time { - numberOfHoursInADay := 24 - if w.truncateTo == "" { - return scheduleTime - } - if w.truncateTo == "h" { - // remove time upto hours - return scheduleTime.Truncate(time.Hour) - } - if w.truncateTo == "d" { - // remove time upto day - return scheduleTime.Truncate(time.Duration(numberOfHoursInADay) * time.Hour) - } - if w.truncateTo == "w" { - truncatedToDay := scheduleTime.Truncate(time.Duration(numberOfHoursInADay) * time.Hour) - // weekday with start of the week as Monday - weekday := scheduleTime.Weekday() - if weekday == 0 { - weekday = 7 - } - daysToSubtract := weekday - time.Monday - - durationToSubtract := time.Duration(daysToSubtract) * time.Duration(numberOfHoursInADay) * time.Hour - return truncatedToDay.Add(-durationToSubtract) - } - if w.truncateTo == "M" { - return time.Date(scheduleTime.Year(), scheduleTime.Month(), 1, 0, 0, 0, 0, time.UTC) - } - return scheduleTime -} - -func (w windowV2) adjustOffset(truncatedTime time.Time) (time.Time, error) { - if w.offset == "" { - return truncatedTime, nil - } - months, nonMonthDurationString, err := monthsAndNonMonthExpression(w.offset) - if err != nil { - return time.Time{}, err - } - - nonMonthDuration, err := time.ParseDuration(nonMonthDurationString) - if err != nil { - return time.Time{}, err - } - return truncatedTime.Add(nonMonthDuration).AddDate(0, months, 0), nil -} - -func (w windowV2) getStartTime(endTime time.Time) (time.Time, error) { - if w.size == "" { - return endTime, nil - } - months, nonMonthDurationString, err := monthsAndNonMonthExpression(w.size) - if err != nil { - return time.Time{}, err - } - nonMonthDuration, err := time.ParseDuration(nonMonthDurationString) - if err != nil { // not expecting this, if this happens due to bad code just return inputTime - return time.Time{}, err - } - return endTime.Add(-nonMonthDuration).AddDate(0, -months, 0), nil -} diff --git a/internal/models/window_v2_test.go b/internal/models/window_v2_test.go deleted file mode 100644 index 5a8dc73fe3..0000000000 --- a/internal/models/window_v2_test.go +++ /dev/null @@ -1,292 +0,0 @@ -package models_test - -import ( - "fmt" - "testing" - "time" - - "github.com/stretchr/testify/assert" - - "github.com/goto/optimus/internal/models" -) - -func TestWindowV2(t *testing.T) { - t.Run("Validate", func(t *testing.T) { - t.Run("should not throw error for window size which is not a positive or an instant time duration", func(t *testing.T) { - validWindowConfigs := []string{"24h", "2h45m", "60s", "45m24h", "", "0", "2M", "45M24h", "45M24h30m"} - for _, config := range validWindowConfigs { - window, err := models.NewWindow(2, "", "", config) - if err != nil { - panic(err) - } - err = window.Validate() - assert.Nil(t, err, fmt.Sprintf("failed for : %s", config)) - } - }) - t.Run("should throw error for window size which is not a valid time duration", func(t *testing.T) { - inValidWindowConfigs := []string{"60S", "60", "2d", "-24h", "-45M24h30m"} - for _, config := range inValidWindowConfigs { - window, err := models.NewWindow(2, "", "", config) - if err != nil { - panic(err) - } - err = window.Validate() - assert.NotNil(t, err, fmt.Sprintf("failed for %s", config)) - } - }) - t.Run("should not throw error for window offset which is not a time duration", func(t *testing.T) { - validOffsetConfigs := []string{"24h", "2h45m", "60s", "45m24h", "0", "", "2M", "45M24h", "45M24h30m", "-45M24h30m"} - for _, config := range validOffsetConfigs { - window, err := models.NewWindow(2, "", config, "") - if err != nil { - panic(err) - } - err = window.Validate() - assert.Nil(t, err, fmt.Sprintf("failed for : %s", config)) - } - }) - t.Run("should throw error for window offset which is not a valid time duration", func(t *testing.T) { - inValidOffsetConfigs := []string{"60S", "60"} - for _, config := range inValidOffsetConfigs { - window, err := models.NewWindow(2, "", config, "") - if err != nil { - panic(err) - } - err = window.Validate() - assert.NotNil(t, err, fmt.Sprintf("failed for %s", config)) - } - }) - t.Run("should not throw error for valid window truncate configs", func(t *testing.T) { - validTruncateConfigs := []string{"h", "d", "w", "M", ""} - for _, config := range validTruncateConfigs { - window, err := models.NewWindow(2, config, "", "") - if err != nil { - panic(err) - } - err = window.Validate() - assert.Nil(t, err, fmt.Sprintf("failed for : %s", config)) - } - }) - t.Run("should throw error for window truncate when it is not a truncate option", func(t *testing.T) { - inValidTruncateConfigs := []string{"s", "a", "ms", "m", "H", "D", "W"} - for _, config := range inValidTruncateConfigs { - window, err := models.NewWindow(2, config, "", "") - if err != nil { - panic(err) - } - err = window.Validate() - assert.NotNil(t, err, fmt.Sprintf("failed for %s", config)) - } - }) - }) - t.Run("GetTimeRange", func(t *testing.T) { - t.Run("should provide start and end time window for the given schedule", func(t *testing.T) { - cases := []struct { - Scenario string - ScheduleTime time.Time - Size string - Offset string - TruncateTo string - ExpectedStartTime time.Time - ExpectedEndTime time.Time - }{ - { - Scenario: "should truncate to the previous hour on truncate to hour", - ScheduleTime: time.Date(2022, 0o7, 0o5, 0o2, 10, 10, 10, time.UTC), - Size: "24h", - Offset: "0", - TruncateTo: "h", - ExpectedStartTime: time.Date(2022, 0o7, 0o4, 0o2, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2022, 0o7, 0o5, 0o2, 0, 0, 0, time.UTC), - }, - { - Scenario: "should not truncate if truncate to is empty", - ScheduleTime: time.Date(2022, 0o7, 0o5, 0o2, 10, 10, 10, time.UTC), - Size: "24h", - Offset: "0", - TruncateTo: "", - ExpectedStartTime: time.Date(2022, 0o7, 0o4, 0o2, 10, 10, 10, time.UTC), - ExpectedEndTime: time.Date(2022, 0o7, 0o5, 0o2, 10, 10, 10, time.UTC), - }, - { - Scenario: "should truncate to the previous day on truncate to day", - ScheduleTime: time.Date(2022, 0o7, 0o5, 0o2, 10, 10, 10, time.UTC), - Size: "24h", - Offset: "0", - TruncateTo: "d", - ExpectedStartTime: time.Date(2022, 0o7, 0o4, 0, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2022, 0o7, 0o5, 0, 0, 0, 0, time.UTC), - }, - { - Scenario: "should truncate to the start of week starting on monday on truncate to week", - ScheduleTime: time.Date(2022, 0o7, 0o5, 0o2, 10, 10, 10, time.UTC), - Size: "24h", - Offset: "0", - TruncateTo: "w", - ExpectedStartTime: time.Date(2022, 0o7, 0o3, 0, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2022, 0o7, 0o4, 0, 0, 0, 0, time.UTC), - }, - { - Scenario: "should truncate to the start of the month on truncate to month", - ScheduleTime: time.Date(2022, 0o7, 0o5, 0o2, 10, 10, 10, time.UTC), - Size: "24h", - Offset: "0", - TruncateTo: "M", - ExpectedStartTime: time.Date(2022, 0o6, 30, 0, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2022, 0o7, 0o1, 0, 0, 0, 0, time.UTC), - }, - { - Scenario: "should not truncate to the previous hour if time is already truncated", - ScheduleTime: time.Date(2022, 0o7, 0o5, 0o2, 0o0, 0o0, 0o0, time.UTC), - Size: "24h", - Offset: "0", - TruncateTo: "h", - ExpectedStartTime: time.Date(2022, 0o7, 0o4, 0o2, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2022, 0o7, 0o5, 0o2, 0, 0, 0, time.UTC), - }, - { - Scenario: "should not truncate to the previous day if time is already truncated", - ScheduleTime: time.Date(2022, 0o7, 0o5, 0o0, 0, 0, 0, time.UTC), - Size: "24h", - Offset: "0", - TruncateTo: "d", - ExpectedStartTime: time.Date(2022, 0o7, 0o4, 0, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2022, 0o7, 0o5, 0, 0, 0, 0, time.UTC), - }, - { - Scenario: "should not truncate to the start of last week if the time is already on monday on truncate to week", - ScheduleTime: time.Date(2022, 0o7, 0o4, 0o0, 0, 0, 0, time.UTC), - Size: "24h", - Offset: "0", - TruncateTo: "w", - ExpectedStartTime: time.Date(2022, 0o7, 0o3, 0, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2022, 0o7, 0o4, 0, 0, 0, 0, time.UTC), - }, - { - Scenario: "should not truncate to the start of month if time is already on beginning of month on truncate to month", - ScheduleTime: time.Date(2022, 0o7, 0o1, 0, 0, 0, 0, time.UTC), - Size: "24h", - Offset: "0", - TruncateTo: "M", - ExpectedStartTime: time.Date(2022, 0o6, 30, 0, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2022, 0o7, 0o1, 0, 0, 0, 0, time.UTC), - }, - { - Scenario: "should not truncate if truncate is empty", - ScheduleTime: time.Date(2022, 0o7, 0o5, 0o2, 10, 10, 10, time.UTC), - Size: "24h", - Offset: "0", - TruncateTo: "", - ExpectedStartTime: time.Date(2022, 0o7, 0o4, 0o2, 10, 10, 10, time.UTC), - ExpectedEndTime: time.Date(2022, 0o7, 0o5, 0o2, 10, 10, 10, time.UTC), - }, - { - Scenario: "should provide window for the configured size in hours and minutes", - ScheduleTime: time.Date(2022, 0o7, 0o5, 0o2, 10, 10, 10, time.UTC), - Size: "24h30m", - Offset: "0", - TruncateTo: "d", - ExpectedStartTime: time.Date(2022, 0o7, 0o3, 23, 30, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2022, 0o7, 0o5, 0, 0, 0, 0, time.UTC), - }, - { - Scenario: "should provide window for the configured size in months", - ScheduleTime: time.Date(2022, 0o7, 0o5, 0o2, 10, 10, 10, time.UTC), - Size: "2M", - Offset: "0", - TruncateTo: "M", - ExpectedStartTime: time.Date(2022, 5, 0o1, 0, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2022, 7, 0o1, 0, 0, 0, 0, time.UTC), - }, - { - Scenario: "should provide window for the configured size in months, hours", - ScheduleTime: time.Date(2022, 0o7, 0o5, 0o2, 10, 10, 10, time.UTC), - Size: "1M24h", - Offset: "0", - TruncateTo: "M", - ExpectedStartTime: time.Date(2022, 0o5, 30, 0, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2022, 0o7, 0o1, 0, 0, 0, 0, time.UTC), - }, - { - Scenario: "should provide window for the configured size in minutes", - ScheduleTime: time.Date(2022, 0o7, 0o5, 0o2, 10, 10, 10, time.UTC), - Size: "60m", - Offset: "0", - TruncateTo: "d", - ExpectedStartTime: time.Date(2022, 0o7, 0o4, 23, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2022, 0o7, 0o5, 0, 0, 0, 0, time.UTC), - }, - { - Scenario: "should not provide any window is size is not configured", - ScheduleTime: time.Date(2022, 0o7, 0o5, 0o2, 10, 10, 10, time.UTC), - Size: "", - Offset: "0", - TruncateTo: "d", - ExpectedStartTime: time.Date(2022, 0o7, 0o5, 0, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2022, 0o7, 0o5, 0, 0, 0, 0, time.UTC), - }, - { - Scenario: "should shift window forward on positive offset", - ScheduleTime: time.Date(2022, 0o7, 0o5, 0o2, 10, 10, 10, time.UTC), - Size: "24h", - Offset: "24h", - TruncateTo: "d", - ExpectedStartTime: time.Date(2022, 0o7, 0o5, 0, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2022, 0o7, 0o6, 0, 0, 0, 0, time.UTC), - }, - { - Scenario: "should shift window backward on negative monthly offset", - ScheduleTime: time.Date(2022, 0o7, 0o5, 0o2, 10, 10, 10, time.UTC), - Size: "1M", - Offset: "-1M", - TruncateTo: "M", - ExpectedStartTime: time.Date(2022, 0o5, 0o1, 0, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2022, 0o6, 0o1, 0, 0, 0, 0, time.UTC), - }, - { - Scenario: "should shift window backward on negative monthly offset with hours", - ScheduleTime: time.Date(2022, 0o7, 0o5, 0o2, 10, 10, 10, time.UTC), - Size: "1M", - Offset: "-1M24h", - TruncateTo: "M", - ExpectedStartTime: time.Date(2022, 0o4, 30, 0, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2022, 0o5, 30, 0, 0, 0, 0, time.UTC), - }, - { - Scenario: "should shift window backward on negative offset", - ScheduleTime: time.Date(2022, 0o7, 0o5, 0o2, 10, 10, 10, time.UTC), - Size: "24h", - Offset: "-24h", - TruncateTo: "d", - ExpectedStartTime: time.Date(2022, 0o7, 0o3, 0, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2022, 0o7, 0o4, 0, 0, 0, 0, time.UTC), - }, - { - Scenario: "should not shift window when offset is not configured", - ScheduleTime: time.Date(2022, 0o7, 0o5, 0o2, 10, 10, 10, time.UTC), - Size: "24h", - Offset: "", - TruncateTo: "d", - ExpectedStartTime: time.Date(2022, 0o7, 0o4, 0, 0, 0, 0, time.UTC), - ExpectedEndTime: time.Date(2022, 0o7, 0o5, 0, 0, 0, 0, time.UTC), - }, - } - for _, sc := range cases { - w, err := models.NewWindow(2, sc.TruncateTo, sc.Offset, sc.Size) - if err != nil { - panic(err) - } - - actualValidateError := w.Validate() - actualStartTime, actualStartTimeError := w.GetStartTime(sc.ScheduleTime) - actualEndTime, actualEndTimeError := w.GetEndTime(sc.ScheduleTime) - - assert.NoError(t, actualValidateError, sc.Scenario) - assert.Equal(t, sc.ExpectedStartTime.String(), actualStartTime.String(), sc.Scenario) - assert.NoError(t, actualStartTimeError, sc.Scenario) - assert.Equal(t, sc.ExpectedEndTime.String(), actualEndTime.String(), sc.Scenario) - assert.NoError(t, actualEndTimeError, sc.Scenario) - } - }) - }) -} diff --git a/internal/store/postgres/job/adapter.go b/internal/store/postgres/job/adapter.go index c14fa838a1..41764f3089 100644 --- a/internal/store/postgres/job/adapter.go +++ b/internal/store/postgres/job/adapter.go @@ -12,7 +12,6 @@ import ( "github.com/goto/optimus/core/job" "github.com/goto/optimus/internal/errors" "github.com/goto/optimus/internal/lib/window" - "github.com/goto/optimus/internal/models" ) const jobDatetimeLayout = "2006-01-02" @@ -247,17 +246,11 @@ func toStorageWindow(windowSpec window.Config) ([]byte, error) { Preset: windowSpec.Preset, } - if windowSpec.GetVersion() == window.NewWindowVersion { - sc := windowSpec.GetSimpleConfig() - w.WindowSize = sc.Size - w.WindowShiftBy = sc.ShiftBy - w.WindowLocation = sc.Location - w.WindowTruncateTo = sc.TruncateTo - } else if windowSpec.Window != nil { - w.WindowSize = windowSpec.Window.GetSize() - w.WindowOffset = windowSpec.Window.GetOffset() - w.WindowTruncateTo = windowSpec.Window.GetTruncateTo() - } + sc := windowSpec.GetSimpleConfig() + w.WindowSize = sc.Size + w.WindowShiftBy = sc.ShiftBy + w.WindowLocation = sc.Location + w.WindowTruncateTo = sc.TruncateTo windowJSON, err := json.Marshal(w) if err != nil { @@ -555,7 +548,7 @@ func fromStorageSpec(jobSpec *Spec) (*job.Spec, error) { return jobSpecBuilder.Build() } -func fromStorageWindow(raw []byte, jobVersion int) (window.Config, error) { +func fromStorageWindow(raw []byte, _ int) (window.Config, error) { var storageWindow Window if err := json.Unmarshal(raw, &storageWindow); err != nil { return window.Config{}, err @@ -569,27 +562,13 @@ func fromStorageWindow(raw []byte, jobVersion int) (window.Config, error) { return window.NewIncrementalConfig(), nil } - if jobVersion == window.NewWindowVersion { - sc := window.SimpleConfig{ - Size: storageWindow.WindowSize, - ShiftBy: storageWindow.WindowShiftBy, - Location: storageWindow.WindowLocation, - TruncateTo: storageWindow.WindowTruncateTo, - } - return window.NewCustomConfigWithTimezone(sc), nil + sc := window.SimpleConfig{ + Size: storageWindow.WindowSize, + ShiftBy: storageWindow.WindowShiftBy, + Location: storageWindow.WindowLocation, + TruncateTo: storageWindow.WindowTruncateTo, } - - w, err := models.NewWindow( - jobVersion, - storageWindow.WindowTruncateTo, - storageWindow.WindowOffset, - storageWindow.WindowSize, - ) - if err != nil { - return window.Config{}, err - } - - return window.NewCustomConfig(w), nil + return window.NewCustomConfigWithTimezone(sc), nil } func fromStorageSchedule(raw []byte) (*job.Schedule, error) { diff --git a/internal/store/postgres/job/job_repository_test.go b/internal/store/postgres/job/job_repository_test.go index 8ad8e0e1cf..06338f4adf 100644 --- a/internal/store/postgres/job/job_repository_test.go +++ b/internal/store/postgres/job/job_repository_test.go @@ -13,7 +13,6 @@ import ( "github.com/goto/optimus/core/resource" "github.com/goto/optimus/core/tenant" "github.com/goto/optimus/internal/lib/window" - "github.com/goto/optimus/internal/models" postgres "github.com/goto/optimus/internal/store/postgres/job" tenantPostgres "github.com/goto/optimus/internal/store/postgres/tenant" "github.com/goto/optimus/tests/setup" @@ -108,9 +107,8 @@ func TestPostgresJobRepository(t *testing.T) { assert.NoError(t, err) jobSchedule, err := job.NewScheduleBuilder(startDate).WithRetry(jobRetry).Build() assert.NoError(t, err) - jobWindow, err := models.NewWindow(jobVersion, "d", "24h", "24h") + customConfig, err := window.NewConfig("1d", "1d", "", "") assert.NoError(t, err) - customConfig := window.NewCustomConfig(jobWindow) jobTaskConfig, err := job.ConfigFrom(map[string]string{"sample_task_key": "sample_value"}) assert.NoError(t, err) taskName, err := job.TaskNameFrom("bq2bq") diff --git a/internal/store/postgres/scheduler/job_repository.go b/internal/store/postgres/scheduler/job_repository.go index f3d1a3a13c..b2e2128319 100644 --- a/internal/store/postgres/scheduler/job_repository.go +++ b/internal/store/postgres/scheduler/job_repository.go @@ -19,7 +19,6 @@ import ( "github.com/goto/optimus/core/tenant" "github.com/goto/optimus/internal/errors" "github.com/goto/optimus/internal/lib/window" - "github.com/goto/optimus/internal/models" "github.com/goto/optimus/internal/utils" ) @@ -142,7 +141,7 @@ type Window struct { Type string } -func fromStorageWindow(raw []byte, jobVersion int) (window.Config, error) { +func fromStorageWindow(raw []byte, _ int) (window.Config, error) { var storageWindow Window if err := json.Unmarshal(raw, &storageWindow); err != nil { return window.Config{}, err @@ -156,27 +155,13 @@ func fromStorageWindow(raw []byte, jobVersion int) (window.Config, error) { return window.NewIncrementalConfig(), nil } - if jobVersion == window.NewWindowVersion { - sc := window.SimpleConfig{ - Size: storageWindow.WindowSize, - ShiftBy: storageWindow.WindowShiftBy, - Location: storageWindow.WindowLocation, - TruncateTo: storageWindow.WindowTruncateTo, - } - return window.NewCustomConfigWithTimezone(sc), nil - } - - w, err := models.NewWindow( - jobVersion, - storageWindow.WindowTruncateTo, - storageWindow.WindowOffset, - storageWindow.WindowSize, - ) - if err != nil { - return window.Config{}, err + sc := window.SimpleConfig{ + Size: storageWindow.WindowSize, + ShiftBy: storageWindow.WindowShiftBy, + Location: storageWindow.WindowLocation, + TruncateTo: storageWindow.WindowTruncateTo, } - - return window.NewCustomConfig(w), nil + return window.NewCustomConfigWithTimezone(sc), nil } type Metadata struct { @@ -467,7 +452,7 @@ func (j *JobRepository) GetAll(ctx context.Context, projectName tenant.ProjectNa jobsMap[jobName].Upstreams.UpstreamJobs = upstreamList } - return utils.MapToList[*scheduler.JobWithDetails](jobsMap), multiError.ToErr() + return utils.MapToList(jobsMap), multiError.ToErr() } func (j *JobRepository) GetJobs(ctx context.Context, projectName tenant.ProjectName, jobs []string) ([]*scheduler.JobWithDetails, error) { @@ -509,7 +494,7 @@ func (j *JobRepository) GetJobs(ctx context.Context, projectName tenant.ProjectN jobsMap[jobName].Upstreams.UpstreamJobs = upstreamList } - return utils.MapToList[*scheduler.JobWithDetails](jobsMap), errors.MultiToError(multiError) + return utils.MapToList(jobsMap), errors.MultiToError(multiError) } func NewJobProviderRepository(pool *pgxpool.Pool) *JobRepository { diff --git a/internal/store/postgres/scheduler/job_repository_test.go b/internal/store/postgres/scheduler/job_repository_test.go index 1eab87e923..0375137711 100644 --- a/internal/store/postgres/scheduler/job_repository_test.go +++ b/internal/store/postgres/scheduler/job_repository_test.go @@ -16,7 +16,6 @@ import ( "github.com/goto/optimus/core/tenant" "github.com/goto/optimus/internal/errors" "github.com/goto/optimus/internal/lib/window" - "github.com/goto/optimus/internal/models" jobRepo "github.com/goto/optimus/internal/store/postgres/job" postgres "github.com/goto/optimus/internal/store/postgres/scheduler" tenantPostgres "github.com/goto/optimus/internal/store/postgres/tenant" @@ -161,8 +160,7 @@ func addJobs(ctx context.Context, t *testing.T, pool *pgxpool.Pool) map[string]* assert.NoError(t, err) jobSchedule, err := job.NewScheduleBuilder(startDate).WithRetry(jobRetry).WithCatchUp(true).WithDependsOnPast(true).Build() assert.NoError(t, err) - jobWindow, err := models.NewWindow(jobVersion, "d", "24h", "24h") - customConfig := window.NewCustomConfig(jobWindow) + customConfig, _ := window.NewConfig("1d", "1d", "", "") assert.NoError(t, err) jobTaskConfig, err := job.ConfigFrom(map[string]string{"sample_task_key": "sample_value"}) assert.NoError(t, err) diff --git a/tests/setup/job.go b/tests/setup/job.go index d2e002f9dc..cd2a95b1c1 100644 --- a/tests/setup/job.go +++ b/tests/setup/job.go @@ -5,7 +5,6 @@ import ( "github.com/goto/optimus/core/resource" "github.com/goto/optimus/core/tenant" "github.com/goto/optimus/internal/lib/window" - "github.com/goto/optimus/internal/models" ) type DummyJobBuilder struct { @@ -52,11 +51,10 @@ func NewDummyJobBuilder() *DummyJobBuilder { } version := 1 - w, err := models.NewWindow(version, "d", "24h", "24h") + windowConfig, err := window.NewConfig("1d", "1d", "", "") if err != nil { panic(err) } - windowConfig := window.NewCustomConfig(w) taskConfig, err := job.ConfigFrom(map[string]string{"sample_task_key": "sample_task_value"}) if err != nil {