| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
|
| |
We sometimes say "✂️ newline after `private`" in a code review (e.g.
https://github.com/rails/rails/pull/18546#discussion_r23188776,
https://github.com/rails/rails/pull/34832#discussion_r244847195).
Now `Layout/EmptyLinesAroundAccessModifier` cop have new enforced style
`EnforcedStyle: only_before` (https://github.com/rubocop-hq/rubocop/pull/7059).
That cop and enforced style will reduce the our code review cost.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Rails 6 introduces retries per-exception, instead of a global count of
retries. Because ActiveJob 5.2 doesn't serialize the execution count
per-exception, when ActiveJob 6.0 picks up an "old" job it can't know
the exception count in the new format.
This can also be an issue if AJ 6.0 serializes a new job with
exception_executions which is later picked up by AJ 5.2, which would
clear exception_executions (since it has no knowledge of it).
Previously we handled this by resetting exception_executions, if it
wasn't defined on a job, which could result in the worst case retrying
the job 2x the times we should.
This commit changes how we handle loading a legacy job: instead of
resetting exception_executions, we instead will always use the global
executions count.
This way, jobs which only have one retry_on (and didn't have a behaviour
change in AJ 6) are backwards-and-forwards-compatible with counts
respected exactly.
Jobs with multiple retry_on will revert to the AJ5.2 behaviour if they
were ever run under AJ5.2.
|
|\
| |
| | |
ActiveJob time argument assertion documentation
|
| | |
|
|/
|
|
|
|
| |
Individual execution counters were introduced in #34352. However
`#determine_delay` which is used to calculate retry delay still uses the
global counter. This commit fixes it.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Motivation:
- Currently we have 2 seperate monkey patches in place for tracking
enqueded time for 2 seperate workers. It seems that activejob could be
a source of truth for how long an item has been enqued so that we can
easily use it for consistent monitoring across workers/apps to ensure
that jobs are running at an acceptable speed.
Changes:
- Add an enqueded at attribute and serilization tooling.
- Add a method to get how long a job has been enqueded for.
- Add a logging item to show how long a job was enqued prior to the
perform method firing.
|
| |
|
|
|
|
|
|
|
|
|
| |
Previously, by extending ActiveJob::TestCase, the test adapter provided
for tests was being used always, in all executions where supposedly
different adapters were being used. As a consequence, some bugs visible
only for some adapters might have gone undetected. This commit changes
that, skipping queue adapters for which we can't test scheduling jobs
with a delay.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
`retry_on` (#34731)
Also, make tests and examples for individual execution counters
clearer, as it wasn't entierly clear what would happen in this case:
```
retry_on CustomException, OtherException, attempts: 3
```
The job would be retried at most 3 times in total, for both
CustomException and OtherException. To have the job retry 3 times at
most for each exception individually, the following retry_on
declarations are necessary:
```
retry_on CustomException, attempts: 3
retry_on OtherException, attempts: 3
```
|
|
|
|
|
|
|
|
|
|
| |
Currently we sometimes find a redundant begin block in code review
(e.g. https://github.com/rails/rails/pull/33604#discussion_r209784205).
I'd like to enable `Style/RedundantBegin` cop to avoid that, since
rescue/else/ensure are allowed inside do/end blocks in Ruby 2.5
(https://bugs.ruby-lang.org/issues/12906), so we'd probably meets with
that situation than before.
|
|\
| |
| | |
Make AJ::Base#enqueue return false if the job wasn't enqueued
|
| | |
|
| |
| |
| |
| |
| |
| |
| | |
Trusting any GlobaID object when deserializing jobs can allow
attackers to access information that should not be accessible to them.
Fix CVE-2018-16476.
|
|\ \
| | |
| | | |
Allow using queue prefix with a default queue name
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Fixes #34366
Currently setting queue_name_prefix will not combine a prefix with the
default_queue_name; it will only affect queue names set with `queue_as`.
With this PR the prefix will affect the default_queue_name as well.
Closes #19831
Currently setting default_queue_name doesn't actually affect the
queue_name default (although default_queue_name does get used if you
pass a falsey `part_name` to `queue_as`). This PR would get
default_queue_name working as expected as well.
Because the queue_name default is now a lambda wrapping the
default_queue_name, rather than the default_queue_name itself, I had to
update one test to use the instance method `#queue_name` (which
`instance_exec`s the value) instead of the class method. I think this
change is OK, since only the instance method is documented.
There was a question about whether we want a `default_queue_name`
configuration. If we want to get rid of it, I would also be happy to
open a PR for that instead. It has been around for a while now, but it
also hasn't really worked for a while now.
r? @matthewd
since you had an opinion about this before
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
* Keep executions for each specific declaration
Fixes #34337
ActiveJob used the global executions counter to control the number of
times a job should be retried. The problem with this approach was that
in case a job raised different exceptions during its executions they
weren't retried the number of times defined by their `attemps` number.
**Example:**
Having the following job:
```ruby
class BuggyJob < ActiveJob::Base
retry_on CustomException, attemps: 3
retry_on OtherException, attempts: 3
end
```
If the job raised `CustomException` in the first two executions and then
it raised `OtherException`, the job wasn't retried anymore because the
global executions counter was already indicating 3 attempts.
With this patch each `retry_on` declaration has its specific counter so
that the first two executions that raise `CustomException` don't affect
the retries count that future exceptions may have.
* Revert "clarifies documentation around the attempts arugment to retry_on"
This reverts commit 86aa8f8c5631f77ed9a208e5107003c01512133e.
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
- That feature is useful to enqueue or assert that jobs got enqueued
or performed based on dynamic conditions.
We will be able to leverage that feature to fix all ActionMailer
assertion helper issue when a Mailer define a custom delivery job
(see next commit).
|
|/ / |
|
| |
| |
| |
| |
| | |
Restore HashWithIndifferentAccess support to
ActiveJob::Arguments.deserialize.
|
|\ \
| |/
|/| |
Improve `enqueue_retry.active_job` message
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Since #33751 was added `enqueue_retry.active_job` instrumentation to
the `retry_on` method, then #33897 moved the instrumentation to `retry_job`
method in order to ensure that this method publish `enqueue_retry.active_job`
notification too.
See related discussion https://github.com/rails/rails/pull/33751#discussion_r214140008
Since `enqueue_retry.active_job` moved to `retry_job`, there is no guarantee
that payload of `enqueue_retry.active_job` would have `:error`.
See test `LoggingTest#test_enqueue_retry_logging_on_retry_job` as
example of that case.
Related to https://github.com/rails/rails/pull/33897#discussion_r219707024
I think we can improve notification of `enqueue_retry.active_job`:
- If there is no `event.payload[:error]`, then publish like
"Retrying RescueJob in 3 seconds." only.
- If `event.payload[:wait]` is `nil`, then publish
"Retrying RescueJob in 0 seconds." instead of "Retrying RescueJob in nil seconds."
- If there is `event.payload[:error]`, then publish
"Retrying RescueJob in 3 seconds, due to a DefaultsError.".
- Change the type of the message from `error` to `info.`
Also, this commit removes part of messages - "The original exception was #{ex.cause.inspect}."
of `enqueue_retry.active_job`, `retry_stopped.active_job`, and `discard.active_job` since I
haven't found it useful. Please let me know whether you agree with that?
|
| |
| |
| |
| |
| |
| |
| |
| | |
`assert_enqueued_with` and `assert_performed_with` return a instantiated
instance of the matching job for further assertion (#21010).
Before this commit the `arguments` method on the returned instance
returns a serialized version of the arguments.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
- When calling `assert_performed_with`/`assert_enqueued_with`, the
+args+ needs to match exactly what the job get passed.
Some jobs can have lot of arguments, or even a simple hash argument
has many key. This is not convenient to test as most tests doesn't
need to check if the arguments matches perfectly.
This PR make it possible to only check if a subset of arguments were
passed to the job.
|
|/ |
|
|
|
|
| |
The test helpers now treat `{ a: 1, b: 2 }` and `{ b: 2, a: 1 }` as equals
|
|
|
|
|
|
|
|
|
|
| |
Also this commit removes `:wait` from payload of
`retry_stopped.active_job`.
Related to https://github.com/rails/rails/pull/33751#discussion_r214140008
Follow up #33751
/cc @kaspth, @rafaelfranca
|
|
|
| |
Fix typo `wiht` -> `with`
|
|
|
|
| |
Without `perform_enqueued_jobs`, job are not executed and assertion is not done.
|
|\
| |
| | |
Add hooks to ActiveJob around retries and discards
|
| | |
|
|/ |
|
|
|
|
|
|
| |
This patch corrects a duplicate method name introduced in #33635.
Also fixes typo in method names.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Example:
```
def test_assert_performed_with
MyJob.perform_later(1,2,3)
perform_enqueued_jobs
assert_performed_with(job: MyJob, args: [1,2,3], queue: 'high')
end
```
Follow up #33626.
|
|
|
|
|
| |
Execution of `assert_performed_jobs`, and `assert_no_performed_jobs`
without a block should respect passed `:except`, `:only`, and `:queue` options.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If the `:queue` option is specified, then only the job(s) enqueued to a specific
queue will not be performed.
Example:
```
def test_assert_no_performed_jobs_with_queue_option
assert_no_performed_jobs queue: :some_queue do
HelloJob.set(queue: :other_queue).perform_later("jeremy")
end
end
```
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If the `:queue` option is specified, then only the job(s) enqueued to a specific
queue will be performed.
Example:
```
def test_assert_performed_jobs_with_queue_option
assert_performed_jobs 1, queue: :some_queue do
HelloJob.set(queue: :some_queue).perform_later("jeremy")
HelloJob.set(queue: :other_queue).perform_later("bogdan")
end
end
```
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If the `:queue` option is specified, then only the job(s) enqueued to
a specific queue will be performed.
Example:
```
def test_perform_enqueued_jobs_with_queue
perform_enqueued_jobs queue: :some_queue do
MyJob.set(queue: :some_queue).perform_later(1, 2, 3) # will be performed
HelloJob.set(queue: :other_queue).perform_later(1, 2, 3) # will not be performed
end
assert_performed_jobs 1
end
```
Follow up #33265
[bogdanvlviv & Jeremy Daer]
|
|
|
|
|
|
|
|
| |
Currently, the execution count increments after deserializes arguments.
Therefore, if an error occurs with deserialize, it retries indefinitely.
In order to prevent this, the count is moved before deserialize.
Fixes #33344.
|
|
|
|
| |
Performs all of the jobs that have been enqueued up to this point in the test.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
It can be asserted that no jobs are enqueued to a specific queue:
```ruby
def test_no_logging
assert_no_enqueued_jobs queue: 'default' do
LoggingJob.set(queue: :some_queue).perform_later
end
end
```
|
|
|
|
|
|
|
|
|
|
|
| |
Rename `in_block_job` to `enqueued_job` since this variable can refer not only
to jobs that were created in the block.
See #33258.
Return back accidentally removed test to activejob/test/cases/test_helper_test.rb
See #33258.
Fix name of tests.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Example of `assert_enqueued_with` with no block
```ruby
def test_assert_enqueued_with
MyJob.perform_later(1,2,3)
assert_enqueued_with(job: MyJob, args: [1,2,3], queue: 'low')
MyJob.set(wait_until: Date.tomorrow.noon).perform_later
assert_enqueued_with(job: MyJob, at: Date.tomorrow.noon)
end
```
Example of `assert_enqueued_email_with` with no block:
```ruby
def test_email
ContactMailer.welcome.deliver_later
assert_enqueued_email_with ContactMailer, :welcome
end
def test_email_with_arguments
ContactMailer.welcome("Hello", "Goodbye").deliver_later
assert_enqueued_email_with ContactMailer, :welcome, args: ["Hello", "Goodbye"]
end
```
Related to #33243
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
`discard_on`
I'm not sure what originally wanted to pass to the argument.
However, as long as see the document added along with the commit, it seems just
to be mistaken that trying to pass the error instance.
https://github.com/rails/rails/pull/30622/files#diff-59beb0189c8c6bc862edf7fdb84ff5a7R64
Fixes #32853
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When a job was just deserialized `arguments` is `nil` and the serialized
arguments are in the `@serialized_arguments` variable. If we try to
serialize this job again the arguments are going to be `nil` instead of
what was serialized.
The test we had was not checking this case because it was deserializing
the job in the same object that had the arguments.
To fix this, when the `@serialized_arguments` are present we return it
instead of the result of the `arguments` serialized.
|
|
|
|
|
|
| |
Record what was the current timezone in effect when the job was
enqueued and then restore when the job is executed in same way
that the current locale is recorded and restored.
|
|\
| |
| | |
Improve ActiveJob custom argument serializers #30941
|
| | |
|