| 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.
|
|
|
|
|
| |
If they're not set we'll still fall back to localhost, but this makes it
possible to run the tests against a remote Postgres / Redis / whatever.
|
| |
|
|
|
|
|
|
|
|
|
| |
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?
|
|\ \
| | |
| | | |
Include deserialized arguments in jobs returned by AJ test helpers
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
`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.
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Since `@mutex.synchronize` is enforced in the `ensure_connection!` method,
there is no need to do so on the caller side.
https://github.com/jondot/sneakers/blob/c1b47f9c5d5a95da728bbe1700795790e4efbb12/lib/sneakers/publisher.rb#L22-L26
Due to this, `ThreadError(deadlock; recursive locking)` has occurred.
|
|/ / |
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
- 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.
|
|/ |
|
|\
| |
| | |
Ignore psqlrc files when executing psql commands
|
| |
| |
| |
| |
| |
| | |
psqlrc files can affect the execution of commands in ways that can hold
up execution by blocking or otherwise cause unexpected side effects and
should best be ignored when using psql programmatically.
|
| |
| |
| |
| | |
The test helpers now treat `{ a: 1, b: 2 }` and `{ b: 2, a: 1 }` as equals
|
|\ \
| |/
|/| |
Follow up #33751
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
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
|
|/
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Currently when executing `bundle exec rake test:integration` under `activejob/`
derectory, it prints helpful info like:
```
(snip)
*** rake aj:integration:async ***
(snip)
*** rake aj:integration:delayed_job ***
(snip)
```
but there is no defined `:aj` scope in `activejob/Rakefile`,
so I think output should be like:
```
(snip)
*** rake test:integration:async ***
(snip)
*** rake test:integration:delayed_job ***
(snip)
```
By the way `rake test:integration` doesn't work if execute it without
prepending `bundle exec` to that command. It is probably what we should
fix too.
|
|
|
| |
Fix typo `wiht` -> `with`
|
|\
| |
| | |
ActiveJob Backburner adapter: fix priority
|
| |
| |
| |
| |
| |
| |
| | |
The priority wasn't being passed from ActiveJob to Backburner, despite
priority being supported. This also brings it inline with the docs,
which mark Backburner as supporting priorities in the "Backend Features"
table: https://api.rubyonrails.org/classes/ActiveJob/QueueAdapters.html
|
|/
|
|
| |
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.
|