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

Derive all internal contexts from Client context #514

Merged
merged 1 commit into from
Aug 8, 2024

Conversation

bgentry
Copy link
Contributor

@bgentry bgentry commented Aug 7, 2024

We were previously using context.Background() in some situations to avoid cancellation of things that really shouldn't be cancelled by the user context cancellation. However, we can now do that with context.WithoutCancel instead in order to preserve context values without any cancellation or deadline.

Fixes #512.

@bgentry bgentry requested a review from brandur August 7, 2024 14:39
We were previously using `context.Background()` in some situations to
avoid cancellation of things that really shouldn't be cancelled by the
user context cancellation. However, we can now do that with
`context.WithoutCancel` instead in order to preserve context values
without any cancellation or deadline.

Fixes #512.
Copy link
Contributor

@brandur brandur left a comment

Choose a reason for hiding this comment

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

Looks great!

@bgentry bgentry merged commit 21d1c23 into master Aug 8, 2024
10 checks passed
@bgentry bgentry deleted the bg-derive-work-fetch-context-from-client-context branch August 8, 2024 02:58
tigrato pushed a commit to gravitational/river that referenced this pull request Dec 18, 2024
We were previously using `context.Background()` in some situations to
avoid cancellation of things that really shouldn't be cancelled by the
user context cancellation. However, we can now do that with
`context.WithoutCancel` instead in order to preserve context values
without any cancellation or deadline.

Fixes riverqueue#512.
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.

Cannot test work that inserts jobs using river.ClientFromContext
2 participants