Skip to content

Extract job executor internal package - #768

Merged
bgentry merged 3 commits into
masterfrom
bg-extract-job-executor-internal-package
Feb 17, 2025
Merged

Extract job executor internal package#768
bgentry merged 3 commits into
masterfrom
bg-extract-job-executor-internal-package

Conversation

@bgentry

@bgentry bgentry commented Feb 17, 2025

Copy link
Copy Markdown
Contributor

This will make it possible to build more advanced rivertest harnesses that simulate real job execution.

One of the main changes in here is to extract some of the top-level error types like JobCancelError into rivertype so that they can be referenced both from the internal/jobexecutor package and the main river package. The only way to make this work well and continue to have the errors be considered "equal" by errors.Is / errors.As is for them to be the same underlying type. As such error types like river.JobCancelError are now aliases to those rivertype.JobCancelError. I don't think this should cause problems, but thought it should be called out.

I also pulled in a standalone commit to add AttemptedBy to the JobUpdate query that's used by tests. Not because it's related to the above, but because it's self-contained and this will help keep the next PR smaller.

Extracted from #766.

@bgentry
bgentry requested a review from brandur February 17, 2025 16:30
This will make it possible to build more advanced rivertest harnesses
that simulate real job execution with a real work context and a real
worker.
@bgentry
bgentry force-pushed the bg-extract-job-executor-internal-package branch from 4daccde to 2ceac4a Compare February 17, 2025 16:31

@brandur brandur left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great. WFM.

@bgentry
bgentry merged commit e4353d3 into master Feb 17, 2025
@bgentry
bgentry deleted the bg-extract-job-executor-internal-package branch February 17, 2025 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants