From f62853cd0957031708c879231d8840cdf9e83dd1 Mon Sep 17 00:00:00 2001 From: Brandur Date: Fri, 10 Nov 2023 17:06:56 -0800 Subject: [PATCH] Rename `RunImmediately` to `RunOnStart` This is another bikeshed issue, but I was thinking about the naming of `RunImmediately` today and I think it might be too ambiguous. It sounds a little too much like it could mean "run immediately when scheduled". I propose we rename it to `RunOnStart` to make it clear that it's expected to run on the scheduler's start (this is also said in the property's docblock). `RunOnStart` is also a little more succinct which is always nice. The repository is technically public now and this is technically a breaking change, but I think it's fair for us to make a few API tweaks in the early days before we've done anything to send the project around (and maybe even a little after). This property in mentioned in our public docs already [1]. If this gets merged I'll follow up by updating it there as well. [1] https://riverqueue.com/docs/periodic-jobs --- client.go | 4 ++-- client_test.go | 4 ++-- example_cron_job_test.go | 2 +- example_periodic_job_test.go | 2 +- internal/maintenance/periodic_job_enqueuer.go | 4 ++-- internal/maintenance/periodic_job_enqueuer_test.go | 6 +++--- periodic.go | 8 ++++---- 7 files changed, 15 insertions(+), 15 deletions(-) diff --git a/client.go b/client.go index 173413a2..b75db21e 100644 --- a/client.go +++ b/client.go @@ -465,8 +465,8 @@ func NewClient[TTx any](driver riverdriver.Driver[TTx], config *Config) (*Client ConstructorFunc: func() (*dbadapter.JobInsertParams, error) { return insertParamsFromArgsAndOptions(periodicJob.constructorFunc()) }, - RunImmediately: opts.RunImmediately, - ScheduleFunc: periodicJob.scheduleFunc.Next, + RunOnStart: opts.RunOnStart, + ScheduleFunc: periodicJob.scheduleFunc.Next, }) } diff --git a/client_test.go b/client_test.go index bd8518b8..2c281d40 100644 --- a/client_test.go +++ b/client_test.go @@ -1038,7 +1038,7 @@ func Test_Client_Maintenance(t *testing.T) { config.PeriodicJobs = []*PeriodicJob{ NewPeriodicJob(cron.Every(15*time.Minute), func() (JobArgs, *InsertOpts) { return periodicJobArgs{}, nil - }, &PeriodicJobOpts{RunImmediately: true}), + }, &PeriodicJobOpts{RunOnStart: true}), } client := runNewTestClient(ctx, t, config) @@ -1070,7 +1070,7 @@ func Test_Client_Maintenance(t *testing.T) { svc := maintenance.GetService[*maintenance.PeriodicJobEnqueuer](client.queueMaintainer) svc.TestSignals.EnteredLoop.WaitOrTimeout() - // No jobs yet because the RunImmediately option was not specified. + // No jobs yet because the RunOnStart option was not specified. jobs, err := queries.JobGetByKind(ctx, client.driver.GetDBPool(), (periodicJobArgs{}).Kind()) require.NoError(t, err) require.Len(t, jobs, 0) diff --git a/example_cron_job_test.go b/example_cron_job_test.go index 19d34573..d4b06e8d 100644 --- a/example_cron_job_test.go +++ b/example_cron_job_test.go @@ -62,7 +62,7 @@ func Example_cronJob() { func() (river.JobArgs, *river.InsertOpts) { return CronJobArgs{}, nil }, - &river.PeriodicJobOpts{RunImmediately: true}, + &river.PeriodicJobOpts{RunOnStart: true}, ), }, Queues: map[string]river.QueueConfig{ diff --git a/example_periodic_job_test.go b/example_periodic_job_test.go index af103c39..2f246e9c 100644 --- a/example_periodic_job_test.go +++ b/example_periodic_job_test.go @@ -55,7 +55,7 @@ func Example_periodicJob() { func() (river.JobArgs, *river.InsertOpts) { return PeriodicJobArgs{}, nil }, - &river.PeriodicJobOpts{RunImmediately: true}, + &river.PeriodicJobOpts{RunOnStart: true}, ), }, Queues: map[string]river.QueueConfig{ diff --git a/internal/maintenance/periodic_job_enqueuer.go b/internal/maintenance/periodic_job_enqueuer.go index 028b438e..851d2493 100644 --- a/internal/maintenance/periodic_job_enqueuer.go +++ b/internal/maintenance/periodic_job_enqueuer.go @@ -34,7 +34,7 @@ func (ts *PeriodicJobEnqueuerTestSignals) Init() { // subpackage. type PeriodicJob struct { ConstructorFunc func() (*dbadapter.JobInsertParams, error) - RunImmediately bool + RunOnStart bool ScheduleFunc func(time.Time) time.Time nextRunAt time.Time // set on service start @@ -120,7 +120,7 @@ func (s *PeriodicJobEnqueuer) Start(ctx context.Context) error { periodicJob.nextRunAt = periodicJob.ScheduleFunc(now) - if periodicJob.RunImmediately { + if periodicJob.RunOnStart { if insertParams, ok := s.insertParamsFromConstructor(ctx, periodicJob.ConstructorFunc); ok { insertParamsMany = append(insertParamsMany, insertParams) } diff --git a/internal/maintenance/periodic_job_enqueuer_test.go b/internal/maintenance/periodic_job_enqueuer_test.go index 42e3efad..7e54edbd 100644 --- a/internal/maintenance/periodic_job_enqueuer_test.go +++ b/internal/maintenance/periodic_job_enqueuer_test.go @@ -112,13 +112,13 @@ func TestPeriodicJobEnqueuer(t *testing.T) { requireNJobs(t, bundle.dbPool, "periodic_job_1500ms", 1) }) - t.Run("RunImmediately", func(t *testing.T) { + t.Run("RunOnStart", func(t *testing.T) { t.Parallel() svc, bundle := setup(t) svc.periodicJobs = []*PeriodicJob{ - {ScheduleFunc: periodicIntervalSchedule(5 * time.Second), ConstructorFunc: jobConstructorFunc("periodic_job_5s"), RunImmediately: true}, + {ScheduleFunc: periodicIntervalSchedule(5 * time.Second), ConstructorFunc: jobConstructorFunc("periodic_job_5s"), RunOnStart: true}, } start := time.Now() @@ -138,7 +138,7 @@ func TestPeriodicJobEnqueuer(t *testing.T) { svc.periodicJobs = []*PeriodicJob{ // skip this insert when it returns nil: - {ScheduleFunc: periodicIntervalSchedule(time.Second), ConstructorFunc: func() (*dbadapter.JobInsertParams, error) { return nil, ErrNoJobToInsert }, RunImmediately: true}, + {ScheduleFunc: periodicIntervalSchedule(time.Second), ConstructorFunc: func() (*dbadapter.JobInsertParams, error) { return nil, ErrNoJobToInsert }, RunOnStart: true}, } require.NoError(t, svc.Start(ctx)) diff --git a/periodic.go b/periodic.go index 6cabe061..abe40bc3 100644 --- a/periodic.go +++ b/periodic.go @@ -29,11 +29,11 @@ type PeriodicJob struct { // PeriodicJobOpts are options for a periodic job. type PeriodicJobOpts struct { - // RunImmediately can be used to indicate that a periodic job should insert - // an initial job as a new scheduler is started. This can be used as a hedge + // RunOnStart can be used to indicate that a periodic job should insert an + // initial job as a new scheduler is started. This can be used as a hedge // for jobs with longer scheduled durations that may not get to expiry // before a new scheduler is elected. - RunImmediately bool + RunOnStart bool } // NewPeriodicJob returns a new PeriodicJob given a schedule and a constructor @@ -53,7 +53,7 @@ type PeriodicJobOpts struct { // are scheduled each time a job's target run time is reached and a new job // inserted. However, each scheduler only retains in-memory state, so anytime a // process quits or a new leader is elected, the whole process starts over -// without regard for the state of the last scheduler. The RunImmediately option +// without regard for the state of the last scheduler. The RunOnStart option // can be used as a hedge to make sure that jobs with long run durations are // guaranteed to occasionally run. func NewPeriodicJob(scheduleFunc PeriodicSchedule, constructorFunc PeriodicJobConstructor, opts *PeriodicJobOpts) *PeriodicJob {