MG-36 - Global ticker for scheduled rules #42

nyagamunene · 2025-01-17T18:53:01Z

What type of PR is this?

This is a feature because it adds the following functionality: it adds a startdatetime variable and adds a filter for fetching rules by schedule and a global ticker that will iterates over rules every minute and check if there are rules that fit criteria it triggers them.

What does this do?

Adds Start date and time
Adds Filter for fetching rules by schedule
Adds Global ticker that will iterate over rules every minute and check if there are rules that fit criteria using 2) and trigger them if there are such rules

Which issue(s) does this PR fix/relate to?

Related Issue Create a global ticker for scheduled rules #36
Resolves Create a global ticker for scheduled rules #36

Have you included tests for your changes?

Yes

Did you document any new/modified feature?

No

Notes

Signed-off-by: nyagamunene <[email protected]>

dborovcanin

Please provide tests for this.

Signed-off-by: nyagamunene <[email protected]>

dborovcanin · 2025-01-22T08:13:57Z

re/service.go

@@ -34,10 +43,100 @@ const (
 	Monthly
 )

+func (rt ReccuringType) String() string {


Move Schedule-related structs and methods to a new schedule.go file.

dborovcanin · 2025-01-22T08:23:34Z

re/service.go

-	RecurringType   ReccuringType `json:"recurring_type"`
-	RecurringPeriod uint          `json:"recurring_period"` // 1 meaning every Recurring value, 2 meaning every other, and so on.
+	StartDateTime   time.Time     `json:"start_datetime"`           // When the schedule becomes active
+	RecurringTime   []time.Time   `json:"recurring_time,omitempty"` // Specific times for the rule to run


Let's use only one time instead of slices here (to simplify scheduler).

dborovcanin · 2025-01-22T08:26:33Z

re/service.go

+	StartDateTime   time.Time     `json:"start_datetime"`           // When the schedule becomes active
+	RecurringTime   []time.Time   `json:"recurring_time,omitempty"` // Specific times for the rule to run
+	RecurringType   ReccuringType `json:"recurring_type"`           // None, Daily, Weekly, Monthly
+	RecurringPeriod uint          `json:"recurring_period"`         // 1 meaning every Recurring value, 2 meaning every other, and so on.


Fix the comment - 1 = every RecurringType, 2 = every other RecurringType...

dborovcanin · 2025-01-22T09:02:15Z

re/service.go

+	RecurringPeriod uint          `json:"recurring_period"`         // 1 meaning every Recurring value, 2 meaning every other, and so on.
+}
+
+func (s Schedule) MarshalJSON() ([]byte, error) {


In the DB, we store it as TIMESTAMP (not TIMESTAMPZ) because we don't want to take DBMS server timezone into account; we'll rather always work with UTC internally. That being said, we need to take care all the inputs are taken with timezone and converted to UTC.

dborovcanin · 2025-01-22T09:03:59Z

re/service.go

@@ -15,6 +17,13 @@ import (
 	lua "github.com/yuin/gopher-lua"
 )

+const (
+	timeFormat = "2006-01-02T15:04"
+	timeZone   = 3 * time.Hour


Why do we use timezone at all?

dborovcanin · 2025-01-22T09:08:14Z

re/service.go

 }

 type re struct {
 	idp    supermq.IDProvider
 	repo   Repository
 	pubSub messaging.PubSub
 	errors chan error
+	ticker *time.Ticker


No need for a ticker here, it's enclosed in StartScheduler.

No need for a ticker here, it's enclosed in StartScheduler.

This is needed here so that I can be able to mock the ticker

dborovcanin · 2025-01-22T09:09:47Z

re/service.go

+func (re *re) newTicker(t time.Duration) *time.Ticker {
+	if re.ticker != nil {
+		return re.ticker
+	}
+	return time.NewTicker(t)
+}


Remove. StartScheduler will be called only once anyway (no need to make it "singleton" in any way).

dborovcanin · 2025-01-22T09:11:41Z

re/postgres/repository.go

 	`

 	listRulesQuery = `
 		SELECT id, name, domain_id, input_channel, input_topic, logic_type, logic_value, output_channel, 
-			output_topic, recurring_time, recurring_type, recurring_period, created_at, created_by, updated_at, updated_by, status
+			output_topic, start_datetime, recurring_time, recurring_type, recurring_period, created_at, created_by, updated_at, updated_by, status


We'll probably need to improve and make this query smarter (so we eliminate some rules already here), but we'll do it in the follow-up PRs.

Signed-off-by: nyagamunene <[email protected]>

dborovcanin · 2025-01-23T09:14:49Z

re/api/endpoints.go

 		rule, err := s.AddRule(ctx, session, req.Rule)
 		if err != nil {
+			fmt.Printf("error is %+v\n", err)


dborovcanin · 2025-01-23T09:16:14Z

re/schedule.go

+
+type Schedule struct {
+	StartDateTime   time.Time     `json:"start_datetime"`          // When the schedule becomes active
+	RecurringTime   time.Time     `json:"recurring_time"`          // Specific time for the rule to run


Rename to Time.

dborovcanin · 2025-01-23T09:17:37Z

re/schedule.go

+type Schedule struct {
+	StartDateTime   time.Time     `json:"start_datetime"`          // When the schedule becomes active
+	RecurringTime   time.Time     `json:"recurring_time"`          // Specific time for the rule to run
+	RecurringType   ReccuringType `json:"recurring_type"`          // None, Daily, Weekly, Monthly


Rename to Recurring.

dborovcanin · 2025-01-23T09:22:30Z

re/schedule.go

+	StartDateTime   time.Time     `json:"start_datetime"`          // When the schedule becomes active
+	RecurringTime   time.Time     `json:"recurring_time"`          // Specific time for the rule to run
+	RecurringType   ReccuringType `json:"recurring_type"`          // None, Daily, Weekly, Monthly
+	RecurringPeriod uint          `json:"recurring_period"`        // Controls how many intervals to skip between executions: 1 runs at every interval, 2 runs at every second interval, etc.


Controls how many intervals to skip between executions: 1 = every Recurring value, 2 = every other Recurring value...

dborovcanin · 2025-01-23T09:25:11Z

re/service.go

 func (re *re) shouldRunRule(rule Rule, startTime time.Time) bool {
-	now := time.Now().Add(timeZone).Truncate(time.Minute)
+	now := time.Now().Truncate(time.Minute)


You can pass only Schedule func (re *re) shouldRun(s Schedule) bool, no need for the whole rule or start time here.

This has not been addressed.

This should be the method attached to the Rule, so we have rule.ShouldRun(time.Time).

dborovcanin · 2025-01-23T09:29:06Z

re/service.go

+	if t.Year() == now.Year() &&
+		t.Month() == now.Month() &&
+		t.Day() == now.Day() &&
+		t.Hour() == now.Hour() &&
+		t.Minute() == now.Minute() {
+		return true


Can you simplify via:

if s.StartDateTime.Equal(now) { return true }

Signed-off-by: nyagamunene <[email protected]>

re/service.go

dborovcanin · 2025-01-24T09:48:04Z

re/service.go

+	case Daily:
+		if s.RecurringPeriod > 0 {
+			daysSinceStart := startTime.Sub(s.StartDateTime).Hours() / 24
+			if int(daysSinceStart)%int(s.RecurringPeriod) == 0 {


Extract period := int(s.RecurringPeriod) before switch.

dborovcanin · 2025-01-24T09:48:19Z

re/service.go

+		}
+	case Monthly:
+		if s.RecurringPeriod > 0 {
+			monthsSinceStart := (startTime.Year()-s.StartDateTime.Year())*12 +


Why are we multiplying by 12?

we are converting from years to months

dborovcanin · 2025-01-24T09:49:01Z

re/service.go

+	switch s.Recurring {
+	case Daily:
+		if s.RecurringPeriod > 0 {
+			daysSinceStart := startTime.Sub(s.StartDateTime).Hours() / 24


Extract 24 and 27 * 7 to constants.

Signed-off-by: nyagamunene <[email protected]>

dborovcanin · 2025-01-24T12:49:06Z

cmd/re/main.go

+		err = svc.StartScheduler(ctx)
+		return err


return svc.StartScheduler(ctx)

dborovcanin · 2025-01-24T12:51:29Z

re/service.go

 func (re *re) shouldRunRule(rule Rule, startTime time.Time) bool {
-	now := time.Now().Add(timeZone).Truncate(time.Minute)
+	now := time.Now().Truncate(time.Minute)


This should be the method attached to the Rule, so we have rule.ShouldRun(time.Time).

Signed-off-by: nyagamunene <[email protected]>

dborovcanin · 2025-01-24T13:57:55Z

re/service.go

+	}
+}
+
+func (r Rule) shouldRunRule(startTime time.Time) bool {


Rename to shouldRun.

Signed-off-by: nyagamunene <[email protected]>

initial implementation

60ab228

Signed-off-by: nyagamunene <[email protected]>

nyagamunene self-assigned this Jan 17, 2025

nyagamunene added 5 commits January 20, 2025 14:28

update main.go file

33d9ff1

Signed-off-by: nyagamunene <[email protected]>

update Reccuring Type method

f8490f2

Signed-off-by: nyagamunene <[email protected]>

add marshalling and unmrshalling for times

d19f729

Signed-off-by: nyagamunene <[email protected]>

fix failing linter

770ec32

Signed-off-by: nyagamunene <[email protected]>

fix postgres storage

cd3b0ba

Signed-off-by: nyagamunene <[email protected]>

nyagamunene marked this pull request as ready for review January 20, 2025 20:52

dborovcanin requested changes Jan 21, 2025

View reviewed changes

nyagamunene added 2 commits January 21, 2025 22:27

add tests for schedular

5df52f8

Signed-off-by: nyagamunene <[email protected]>

fix failing linter

64684db

Signed-off-by: nyagamunene <[email protected]>

dborovcanin requested changes Jan 22, 2025

View reviewed changes

nyagamunene added 2 commits January 23, 2025 02:26

address comments

46f21f8

Signed-off-by: nyagamunene <[email protected]>

fix linter

c0319fd

Signed-off-by: nyagamunene <[email protected]>

dborovcanin requested changes Jan 23, 2025

View reviewed changes

nyagamunene added 2 commits January 23, 2025 13:15

address comments

49c3582

Signed-off-by: nyagamunene <[email protected]>

fix linter

ace0fc1

Signed-off-by: nyagamunene <[email protected]>

dborovcanin requested changes Jan 24, 2025

View reviewed changes

nyagamunene added 2 commits January 24, 2025 12:52

refactor should run

0b53aba

Signed-off-by: nyagamunene <[email protected]>

refactor how rule are ran

67ebe9a

Signed-off-by: nyagamunene <[email protected]>

dborovcanin requested changes Jan 24, 2025

View reviewed changes

address comments

68bc7a0

Signed-off-by: nyagamunene <[email protected]>

dborovcanin reviewed Jan 24, 2025

View reviewed changes

re/service.go Outdated

}

}

func (r Rule) shouldRunRule(startTime time.Time) bool {

Copy link

Contributor

dborovcanin Jan 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename to shouldRun.

rename method

d771f2d

Signed-off-by: nyagamunene <[email protected]>

dborovcanin approved these changes Jan 24, 2025

View reviewed changes

dborovcanin merged commit 1b6b7f5 into absmach:main Jan 24, 2025
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MG-36 - Global ticker for scheduled rules #42

MG-36 - Global ticker for scheduled rules #42

nyagamunene commented Jan 17, 2025 •

edited

Loading

dborovcanin left a comment

dborovcanin Jan 22, 2025

dborovcanin Jan 22, 2025 •

edited

Loading

dborovcanin Jan 22, 2025

dborovcanin Jan 22, 2025

dborovcanin Jan 22, 2025

dborovcanin Jan 22, 2025

nyagamunene Jan 22, 2025

dborovcanin Jan 22, 2025

dborovcanin Jan 22, 2025

dborovcanin Jan 23, 2025

dborovcanin Jan 23, 2025

dborovcanin Jan 23, 2025

dborovcanin Jan 23, 2025

dborovcanin Jan 23, 2025

dborovcanin Jan 24, 2025

dborovcanin Jan 24, 2025

dborovcanin Jan 23, 2025

dborovcanin Jan 24, 2025

dborovcanin Jan 24, 2025

nyagamunene Jan 24, 2025

dborovcanin Jan 24, 2025

dborovcanin Jan 24, 2025

dborovcanin Jan 24, 2025

dborovcanin Jan 24, 2025

MG-36 - Global ticker for scheduled rules #42

MG-36 - Global ticker for scheduled rules #42

Conversation

nyagamunene commented Jan 17, 2025 • edited Loading

What type of PR is this?

What does this do?

Which issue(s) does this PR fix/relate to?

Have you included tests for your changes?

Did you document any new/modified feature?

Notes

dborovcanin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dborovcanin Jan 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nyagamunene commented Jan 17, 2025 •

edited

Loading

dborovcanin Jan 22, 2025 •

edited

Loading