Allow to specify parent class for active record #238
Conversation
lib/good_job/lockable.rb
Outdated
| @@ -143,7 +143,7 @@ def with_advisory_lock | |||
| def supports_cte_materialization_specifiers? | |||
| return @_supports_cte_materialization_specifiers if defined?(@_supports_cte_materialization_specifiers) | |||
|
|
|||
| @_supports_cte_materialization_specifiers = ActiveRecord::Base.connection.postgresql_version >= 120000 | |||
| @_supports_cte_materialization_specifiers = Job.connection.postgresql_version >= 120000 | |||
There was a problem hiding this comment.
The GoodJob::Lockable class is intended to be a generic ActiveRecord mixin, so this hopefully can be:
| @_supports_cte_materialization_specifiers = Job.connection.postgresql_version >= 120000 | |
| @_supports_cte_materialization_specifiers = self.class.connection.postgresql_version >= 120000 |
There was a problem hiding this comment.
hmm, this doesn't work somehow
There was a problem hiding this comment.
Whoops. This is within class_methods so it just needs to be: connection.postgresql_version >= 120000
lib/good_job/lockable.rb
Outdated
| @@ -158,7 +158,7 @@ def advisory_lock | |||
| WHERE pg_try_advisory_lock(('x'||substr(md5($1 || $2::text), 1, 16))::bit(64)::bigint) | |||
| SQL | |||
| binds = [[nil, self.class.table_name], [nil, send(self.class.primary_key)]] | |||
| ActiveRecord::Base.connection.exec_query(pg_or_jdbc_query(query), 'GoodJob::Lockable Advisory Lock', binds).any? | |||
| Job.connection.exec_query(pg_or_jdbc_query(query), 'GoodJob::Lockable Advisory Lock', binds).any? | |||
There was a problem hiding this comment.
Same here:
| Job.connection.exec_query(pg_or_jdbc_query(query), 'GoodJob::Lockable Advisory Lock', binds).any? | |
| self.class.connection.exec_query(pg_or_jdbc_query(query), 'GoodJob::Lockable Advisory Lock', binds).any? |
| @@ -2,7 +2,7 @@ module GoodJob | |||
| # | |||
| # Represents a request to perform an +ActiveJob+ job. | |||
| # | |||
| class Job < ActiveRecord::Base | |||
| class Job < Object.const_get(GoodJob.active_record_parent_class) | |||
lib/good_job.rb
Outdated
| @@ -17,6 +17,9 @@ | |||
| # | |||
| # +GoodJob+ is the top-level namespace and exposes configuration attributes. | |||
| module GoodJob | |||
| # TODO: explain me | |||
There was a problem hiding this comment.
| # TODO: explain me | |
| # @!attribute [rw] active_record_parent_class | |
| # @!scope class | |
| # The ActiveRecord parent class inherited by +GoodJob::Job+ (default: +ActiveRecord::Base+). | |
| # Use this when using multiple databases or other custom ActiveRecord configuration. | |
| # @return [ActiveRecord::Base] | |
| # @example Change the base class: | |
| # GoodJob.active_record_parent_class = "CustomApplicationRecord" |
|
@bensheldon Anything more I should add to make it merged? I was thinking about some spec, but it might be tricky to test inheritance as the job file is already loaded on spec run. |
|
@morgoth I agree about the difficulty of testing. It seems like it will either work, or fail across the board. Let's move it forward and see what we learn. |
|
I forgot that this should also be documented in the Readme. Done in 9eac0be |
|
This has been released in |
Refs #236
POC