Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The ScheduledAt insert option doesn't work from JobArgsWithInsertOpts #484

Closed
dbhoot opened this issue Jul 30, 2024 · 2 comments · Fixed by #487
Closed

The ScheduledAt insert option doesn't work from JobArgsWithInsertOpts #484

dbhoot opened this issue Jul 30, 2024 · 2 comments · Fixed by #487
Assignees
Labels
bug Something isn't working

Comments

@dbhoot
Copy link

dbhoot commented Jul 30, 2024

According to the docs, the worker args should be able to provide default insert options by implementing the InsertOps function. This includes the ScheduledAt attribute. The comments/docs indicate the scheduled at is normally meant to be provided explicitly, however it should work both ways (because docs say it will work).

        // ScheduledAt is a time in future at which to schedule the job (i.e. in
	// cases where it shouldn't be run immediately). The job is guaranteed not
	// to run before this time, but may run slightly after depending on the
	// number of other scheduled jobs and how busy the queue is.
	//
	// Use of this option generally only makes sense when passing options into
	// Insert rather than when a job args struct is implementing
	// JobArgsWithInsertOpts, however, it will work in both cases.
	ScheduledAt time.Time

However, my reading of client.go (see link) shows that the scheduled at insert options is only respected when passed explicitly on insert. The ScheduledAt field is ignored when coming from InsertOps function on the args being inserted.

https://github.com/riverqueue/river/blob/v0.10.1/client.go#L1212

My work-around is to call InsertOpts() on the args explicitly and pass a reference on every insert

@bgentry bgentry self-assigned this Jul 31, 2024
@bgentry bgentry added the bug Something isn't working label Jul 31, 2024
bgentry added a commit that referenced this issue Jul 31, 2024
This allows for job arg definitions to utilize custom logic at the args
level for determining when the job should be scheduled.

Fixes #484.
bgentry added a commit that referenced this issue Jul 31, 2024
This allows for job arg definitions to utilize custom logic at the args
level for determining when the job should be scheduled.

Fixes #484.
@bgentry
Copy link
Contributor

bgentry commented Jul 31, 2024

@dbhoot thanks for the report and for digging into the code to find the issue! I've fixed this in #487 so that if any non-zero time is returned for ScheduledAt via JobArgsWithInsertOpts, it will be utilized (though the insertion-time InsertOpts still takes precedence if set).

bgentry added a commit that referenced this issue Jul 31, 2024
This allows for job arg definitions to utilize custom logic at the args
level for determining when the job should be scheduled.

Fixes #484.
bgentry added a commit that referenced this issue Jul 31, 2024
This allows for job arg definitions to utilize custom logic at the args
level for determining when the job should be scheduled.

Fixes #484.
@dbhoot
Copy link
Author

dbhoot commented Jul 31, 2024

@bgentry thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants