Remove assertion that jobs are never nested in the ActiveJob extension #369
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This assertion is untrue in the Rails ActiveJob tests, and has been overridden in the Rails repo, so it never runs anyway.
oeoeaio
reviewed
Aug 3, 2022
| # make sense to run a worker inside of a job, but even so, assert that | ||
| # nothing absurd is going on. | ||
| Que.assert NilClass, Thread.current[:que_current_job] | ||
|
|
There was a problem hiding this comment.
This method has only been overridden by an ActiveJob test support helper specifically to prevent this assertion from making the Rails test suite fail. This assertion still runs in production on Rails.
It's possible that removing the assertion is ok, but I would like to have a better grasp as to why it was added in the first place before we do. If there is some logic downstream from this point that depends on the fact that jobs are not nested, then removing the assertion may make the Rails test suite pass but could break que.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This assertion is untrue in the Rails ActiveJob tests, and has been overridden in the Rails repo, so it never runs anyway.
@ZimbiX indicated that we should have some discussion about this to figure out the best approach, so I'm mostly just submitting this PR to stimulate the discussion.
Closes #329