From 05471e26106779e51aa2c51946db50f5ebecf024 Mon Sep 17 00:00:00 2001 From: Arnaud Lachaume Date: Mon, 31 May 2021 12:30:30 +0200 Subject: [PATCH] add support for ruby 3.0.x - fixes #27 --- .github/workflows/test.yml | 2 + examples/rails/.ruby-version | 2 +- examples/sinatra/app.rb | 60 +++++++++---------- lib/cloudtasker/backend/google_cloud_task.rb | 16 ++--- lib/cloudtasker/backend/memory_task.rb | 2 +- lib/cloudtasker/backend/redis_task.rb | 10 +++- lib/cloudtasker/cli.rb | 8 +-- lib/cloudtasker/cloud_task.rb | 4 +- lib/cloudtasker/cron/schedule.rb | 10 ++-- lib/cloudtasker/redis_client.rb | 7 ++- lib/cloudtasker/unique_job/job.rb | 5 +- .../unique_job/middleware/client.rb | 1 + lib/cloudtasker/worker.rb | 6 +- lib/cloudtasker/worker_wrapper.rb | 2 +- .../cloudtasker_adapter/job_wrapper_spec.rb | 2 +- .../backend/google_cloud_task_spec.rb | 2 +- spec/cloudtasker/backend/memory_task_spec.rb | 10 ++-- spec/cloudtasker/backend/redis_task_spec.rb | 20 +++---- spec/cloudtasker/cloud_task_spec.rb | 12 ++-- spec/cloudtasker/cron/schedule_spec.rb | 2 +- spec/cloudtasker/worker_spec.rb | 4 +- spec/cloudtasker/worker_wrapper_spec.rb | 4 +- spec/support/test_middleware.rb | 4 +- 23 files changed, 101 insertions(+), 94 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c3a16fd7..8bbca3fd 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -14,6 +14,8 @@ jobs: ruby: - '2.5.x' - '2.6.x' + - '2.7.x' + - '3.0.x' appraisal: - 'google-cloud-tasks-1.0' - 'google-cloud-tasks-1.1' diff --git a/examples/rails/.ruby-version b/examples/rails/.ruby-version index 80d02f91..bff6ce5c 100644 --- a/examples/rails/.ruby-version +++ b/examples/rails/.ruby-version @@ -1 +1 @@ -ruby-2.5.5 +ruby-2.7.1 diff --git a/examples/sinatra/app.rb b/examples/sinatra/app.rb index b1ba6c8d..418a690d 100644 --- a/examples/sinatra/app.rb +++ b/examples/sinatra/app.rb @@ -15,35 +15,33 @@ end post '/cloudtasker/run' do - begin - # Authenticate request - Cloudtasker::Authenticator.verify!(request.env['HTTP_AUTHORIZATION'].to_s.split(' ').last) - - # Capture content and decode content - content = request.body.read - content = Base64.decode64(content) if request.env['HTTP_CONTENT_TRANSFER_ENCODING'].to_s.downcase == 'base64' - - # Format job payload - payload = JSON.parse(content) - .merge( - job_retries: request.env[Cloudtasker::Config::RETRY_HEADER].to_i, - task_id: request.env[Cloudtasker::Config::TASK_ID_HEADER] - ) - - # Process payload - Cloudtasker::WorkerHandler.execute_from_payload!(payload) - return 204 - rescue Cloudtasker::DeadWorkerError - # 205: job will NOT be retried - return 205 - rescue Cloudtasker::AuthenticationError - # 401: Unauthorized - return 401 - rescue Cloudtasker::InvalidWorkerError - # 404: Job will be retried - return 404 - rescue StandardError - # 422: Job will be retried - return 423 - end + # Authenticate request + Cloudtasker::Authenticator.verify!(request.env['HTTP_AUTHORIZATION'].to_s.split(' ').last) + + # Capture content and decode content + content = request.body.read + content = Base64.decode64(content) if request.env['HTTP_CONTENT_TRANSFER_ENCODING'].to_s.downcase == 'base64' + + # Format job payload + payload = JSON.parse(content) + .merge( + job_retries: request.env[Cloudtasker::Config::RETRY_HEADER].to_i, + task_id: request.env[Cloudtasker::Config::TASK_ID_HEADER] + ) + + # Process payload + Cloudtasker::WorkerHandler.execute_from_payload!(payload) + return 204 +rescue Cloudtasker::DeadWorkerError + # 205: job will NOT be retried + return 205 +rescue Cloudtasker::AuthenticationError + # 401: Unauthorized + return 401 +rescue Cloudtasker::InvalidWorkerError + # 404: Job will be retried + return 404 +rescue StandardError + # 422: Job will be retried + return 423 end diff --git a/lib/cloudtasker/backend/google_cloud_task.rb b/lib/cloudtasker/backend/google_cloud_task.rb index c02d60f8..0c04ef68 100644 --- a/lib/cloudtasker/backend/google_cloud_task.rb +++ b/lib/cloudtasker/backend/google_cloud_task.rb @@ -12,28 +12,30 @@ class GoogleCloudTask # # Create the queue configured in Cloudtasker if it does not already exist. # - # @param [String] queue_name The relative name of the queue. + # @param [String] :name The queue name + # @param [Integer] :concurrency The queue concurrency + # @param [Integer] :retries The number of retries for the queue # # @return [Google::Cloud::Tasks::V2beta3::Queue] The queue # - def self.setup_queue(**opts) + def self.setup_queue(name: nil, concurrency: nil, retries: nil) # Build full queue path - queue_name = opts[:name] || Cloudtasker::Config::DEFAULT_JOB_QUEUE + queue_name = name || Cloudtasker::Config::DEFAULT_JOB_QUEUE full_queue_name = queue_path(queue_name) # Try to get existing queue client.get_queue(full_queue_name) rescue Google::Gax::RetryError # Extract options - concurrency = (opts[:concurrency] || Cloudtasker::Config::DEFAULT_QUEUE_CONCURRENCY).to_i - retries = (opts[:retries] || Cloudtasker::Config::DEFAULT_QUEUE_RETRIES).to_i + queue_concurrency = (concurrency || Cloudtasker::Config::DEFAULT_QUEUE_CONCURRENCY).to_i + queue_retries = (retries || Cloudtasker::Config::DEFAULT_QUEUE_RETRIES).to_i # Create queue on 'not found' error client.create_queue( client.location_path(config.gcp_project_id, config.gcp_location_id), name: full_queue_name, - retry_config: { max_attempts: retries }, - rate_limits: { max_concurrent_dispatches: concurrency } + retry_config: { max_attempts: queue_retries }, + rate_limits: { max_concurrent_dispatches: queue_concurrency } ) end diff --git a/lib/cloudtasker/backend/memory_task.rb b/lib/cloudtasker/backend/memory_task.rb index b8804cab..31b49f34 100644 --- a/lib/cloudtasker/backend/memory_task.rb +++ b/lib/cloudtasker/backend/memory_task.rb @@ -62,7 +62,7 @@ def self.create(payload) payload = payload.merge(schedule_time: payload[:schedule_time].to_i) # Save task - task = new(payload.merge(id: id)) + task = new(**payload.merge(id: id)) queue << task # Execute task immediately if in testing and inline mode enabled diff --git a/lib/cloudtasker/backend/redis_task.rb b/lib/cloudtasker/backend/redis_task.rb index d89cecdf..8472cc48 100644 --- a/lib/cloudtasker/backend/redis_task.rb +++ b/lib/cloudtasker/backend/redis_task.rb @@ -89,7 +89,7 @@ def self.create(payload) # Save job redis.write(key(id), payload) redis.sadd(key, id) - new(payload.merge(id: id)) + new(**payload.merge(id: id)) end # @@ -103,7 +103,7 @@ def self.find(id) gid = key(id) return nil unless (payload = redis.fetch(gid)) - new(payload.merge(id: id)) + new(**payload.merge(id: id)) end # @@ -172,8 +172,12 @@ def gid # Retry the task later. # # @param [Integer] interval The delay in seconds before retrying the task + # @param [Hash] opts Additional options + # @option opts [Boolean] :is_error Increase number of retries. Default to true. # - def retry_later(interval, is_error: true) + def retry_later(interval, opts = {}) + is_error = opts.to_h.fetch(:is_error, true) + redis.write( gid, retries: is_error ? retries + 1 : retries, diff --git a/lib/cloudtasker/cli.rb b/lib/cloudtasker/cli.rb index cfb4219e..32a20f9d 100644 --- a/lib/cloudtasker/cli.rb +++ b/lib/cloudtasker/cli.rb @@ -124,11 +124,9 @@ def setup_signals(write_pipe) # USR1 and USR2 don't work on the JVM sigs << 'USR2' unless jruby? sigs.each do |sig| - begin - trap(sig) { write_pipe.puts(sig) } - rescue ArgumentError - puts "Signal #{sig} not supported" - end + trap(sig) { write_pipe.puts(sig) } + rescue ArgumentError + puts "Signal #{sig} not supported" end end diff --git a/lib/cloudtasker/cloud_task.rb b/lib/cloudtasker/cloud_task.rb index 43928407..770e2023 100644 --- a/lib/cloudtasker/cloud_task.rb +++ b/lib/cloudtasker/cloud_task.rb @@ -37,7 +37,7 @@ def self.backend # def self.find(id) payload = backend.find(id)&.to_h - payload ? new(payload) : nil + payload ? new(**payload) : nil end # @@ -51,7 +51,7 @@ def self.create(payload) raise MaxTaskSizeExceededError if payload.to_json.bytesize > Config::MAX_TASK_SIZE resp = backend.create(payload)&.to_h - resp ? new(resp) : nil + resp ? new(**resp) : nil end # diff --git a/lib/cloudtasker/cron/schedule.rb b/lib/cloudtasker/cron/schedule.rb index 55f12837..0fed533a 100644 --- a/lib/cloudtasker/cron/schedule.rb +++ b/lib/cloudtasker/cron/schedule.rb @@ -93,7 +93,7 @@ def self.create(**opts) def self.find(id) return nil unless (schedule_config = redis.fetch(key(id))) - new(schedule_config) + new(**schedule_config) end # @@ -251,9 +251,9 @@ def next_time(*args) # # Buld edit the object attributes. # - # @param [Hash] **opts The attributes to edit. + # @param [Hash] opts The attributes to edit. # - def assign_attributes(**opts) + def assign_attributes(opts) opts .select { |k, _| instance_variables.include?("@#{k}".to_sym) } .each { |k, v| instance_variable_set("@#{k}", v) } @@ -262,9 +262,9 @@ def assign_attributes(**opts) # # Edit the object attributes and save the object in Redis. # - # @param [Hash] **opts The attributes to edit. + # @param [Hash] opts The attributes to edit. # - def update(**opts) + def update(opts) assign_attributes(opts) save end diff --git a/lib/cloudtasker/redis_client.rb b/lib/cloudtasker/redis_client.rb index 5775d006..d3cca221 100644 --- a/lib/cloudtasker/redis_client.rb +++ b/lib/cloudtasker/redis_client.rb @@ -136,14 +136,15 @@ def search(pattern) # Delegate all methods to the redis client. # # @param [String, Symbol] name The method to delegate. - # @param [Array] *args The list of method arguments. + # @param [Array] *args The list of method positional arguments. + # @param [Hash] *kwargs The list of method keyword arguments. # @param [Proc] &block Block passed to the method. # # @return [Any] The method return value # - def method_missing(name, *args, &block) + def method_missing(name, *args, **kwargs, &block) if Redis.method_defined?(name) - client.with { |c| c.send(name, *args, &block) } + client.with { |c| c.send(name, *args, **kwargs, &block) } else super end diff --git a/lib/cloudtasker/unique_job/job.rb b/lib/cloudtasker/unique_job/job.rb index 19d7b8fe..df636b5b 100644 --- a/lib/cloudtasker/unique_job/job.rb +++ b/lib/cloudtasker/unique_job/job.rb @@ -14,10 +14,11 @@ class Job # Build a new instance of the class. # # @param [Cloudtasker::Worker] worker The worker at hand + # @param [Hash] worker The worker options # - def initialize(worker, **kwargs) + def initialize(worker, opts = {}) @worker = worker - @call_opts = kwargs + @call_opts = opts end # diff --git a/lib/cloudtasker/unique_job/middleware/client.rb b/lib/cloudtasker/unique_job/middleware/client.rb index 419b0da9..16a57576 100644 --- a/lib/cloudtasker/unique_job/middleware/client.rb +++ b/lib/cloudtasker/unique_job/middleware/client.rb @@ -3,6 +3,7 @@ module Cloudtasker module UniqueJob module Middleware + # TODO: kwargs to job otherwise it won't get the time_at # Client middleware, invoked when jobs are scheduled class Client def call(worker, **_kwargs) diff --git a/lib/cloudtasker/worker.rb b/lib/cloudtasker/worker.rb index 24ee7df3..b18e0044 100644 --- a/lib/cloudtasker/worker.rb +++ b/lib/cloudtasker/worker.rb @@ -47,7 +47,7 @@ def self.from_hash(hash) return nil unless worker_klass.include?(self) # Return instantiated worker - worker_klass.new(payload.slice(:job_queue, :job_args, :job_id, :job_meta, :job_retries, :task_id)) + worker_klass.new(**payload.slice(:job_queue, :job_args, :job_id, :job_meta, :job_retries, :task_id)) rescue NameError nil end @@ -121,7 +121,7 @@ def perform_at(time_at, *args) # @return [Cloudtasker::CloudTask] The Google Task response # def schedule(args: nil, time_in: nil, time_at: nil, queue: nil) - new(job_args: args, job_queue: queue).schedule({ interval: time_in, time_at: time_at }.compact) + new(job_args: args, job_queue: queue).schedule(**{ interval: time_in, time_at: time_at }.compact) end # @@ -239,7 +239,7 @@ def schedule_time(interval: nil, time_at: nil) # def schedule(**args) # Evaluate when to schedule the job - time_at = schedule_time(args) + time_at = schedule_time(**args) # Schedule job through client middlewares Cloudtasker.config.client_middleware.invoke(self, time_at: time_at) do diff --git a/lib/cloudtasker/worker_wrapper.rb b/lib/cloudtasker/worker_wrapper.rb index 82548e1c..61f874ed 100644 --- a/lib/cloudtasker/worker_wrapper.rb +++ b/lib/cloudtasker/worker_wrapper.rb @@ -27,7 +27,7 @@ class WorkerWrapper # def initialize(worker_name:, **opts) @worker_name = worker_name - super(opts) + super(**opts) end # diff --git a/spec/active_job/queue_adapters/cloudtasker_adapter/job_wrapper_spec.rb b/spec/active_job/queue_adapters/cloudtasker_adapter/job_wrapper_spec.rb index f47efce7..98dfcd1e 100644 --- a/spec/active_job/queue_adapters/cloudtasker_adapter/job_wrapper_spec.rb +++ b/spec/active_job/queue_adapters/cloudtasker_adapter/job_wrapper_spec.rb @@ -6,7 +6,7 @@ include_context 'of Cloudtasker ActiveJob instantiation' subject :worker do - described_class.new(example_job_wrapper_args.merge(task_id: '00000001')) + described_class.new(**example_job_wrapper_args.merge(task_id: '00000001')) end let :example_unreconstructed_job_serialization do diff --git a/spec/cloudtasker/backend/google_cloud_task_spec.rb b/spec/cloudtasker/backend/google_cloud_task_spec.rb index 2283d01d..c4071647 100644 --- a/spec/cloudtasker/backend/google_cloud_task_spec.rb +++ b/spec/cloudtasker/backend/google_cloud_task_spec.rb @@ -35,7 +35,7 @@ let(:client) { instance_double('Google::Cloud::Tasks::V2beta3::Task') } describe '.setup_queue' do - subject { described_class.setup_queue(opts) } + subject { described_class.setup_queue(**opts) } let(:opts) { { name: relative_queue, concurrency: 20, retries: 100 } } let(:queue) { instance_double('Google::Cloud::Tasks::V2beta3::Queue') } diff --git a/spec/cloudtasker/backend/memory_task_spec.rb b/spec/cloudtasker/backend/memory_task_spec.rb index 22d4c8ba..0330c2e7 100644 --- a/spec/cloudtasker/backend/memory_task_spec.rb +++ b/spec/cloudtasker/backend/memory_task_spec.rb @@ -41,9 +41,9 @@ let(:worker_name) { 'TestWorker' } let(:worker_name2) { 'TestWorker2' } let(:task_id) { '1234' } - let(:task) { described_class.new(job_payload.merge(id: task_id)) } + let(:task) { described_class.new(**job_payload.merge(id: task_id)) } let(:task_id2) { '2434' } - let(:task2) { described_class.new(job_payload2.merge(id: task_id2)) } + let(:task2) { described_class.new(**job_payload2.merge(id: task_id2)) } before { described_class.clear } @@ -160,7 +160,7 @@ end describe '.new' do - subject { described_class.new(job_payload.merge(id: id)) } + subject { described_class.new(**job_payload.merge(id: id)) } let(:id) { '123' } let(:expected_attrs) do @@ -251,11 +251,11 @@ subject { task } context 'with same id' do - it { is_expected.to eq(described_class.new(job_payload.merge(id: task_id))) } + it { is_expected.to eq(described_class.new(**job_payload.merge(id: task_id))) } end context 'with different id' do - it { is_expected.not_to eq(described_class.new(job_payload.merge(id: task_id + 'a'))) } + it { is_expected.not_to eq(described_class.new(**job_payload.merge(id: task_id + 'a'))) } end context 'with different object' do diff --git a/spec/cloudtasker/backend/redis_task_spec.rb b/spec/cloudtasker/backend/redis_task_spec.rb index b64600be..ba469c2e 100644 --- a/spec/cloudtasker/backend/redis_task_spec.rb +++ b/spec/cloudtasker/backend/redis_task_spec.rb @@ -22,7 +22,7 @@ } end let(:task_id) { '1234' } - let(:task) { described_class.new(job_payload.merge(id: task_id)) } + let(:task) { described_class.new(**job_payload.merge(id: task_id)) } describe '.redis' do subject { described_class.redis } @@ -69,9 +69,9 @@ let(:queue) { nil } let(:tasks) do [ - described_class.new(job_payload.merge(id: 1, queue: 'critical')), - described_class.new(job_payload.merge(id: 2, queue: 'default')), - described_class.new(job_payload.merge(id: 3, schedule_time: Time.now + 3600)) + described_class.new(**job_payload.merge(id: 1, queue: 'critical')), + described_class.new(**job_payload.merge(id: 2, queue: 'default')), + described_class.new(**job_payload.merge(id: 3, schedule_time: Time.now + 3600)) ] end @@ -94,8 +94,8 @@ let(:queue) { 'some-queue' } let(:tasks) do [ - described_class.new(job_payload.merge(id: 1)), - described_class.new(job_payload.merge(id: 2)) + described_class.new(**job_payload.merge(id: 1)), + described_class.new(**job_payload.merge(id: 2)) ] end @@ -124,7 +124,7 @@ describe '.find' do subject { described_class.find(task_id) } - let(:expected_record) { described_class.new(job_payload.merge(id: task_id)) } + let(:expected_record) { described_class.new(**job_payload.merge(id: task_id)) } context 'with record found' do before { allow(SecureRandom).to receive(:uuid).and_return(task_id) } @@ -153,7 +153,7 @@ end describe '.new' do - subject { described_class.new(args) } + subject { described_class.new(**args) } let(:id) { '123' } let(:args) { job_payload.merge(id: id) } @@ -279,11 +279,11 @@ subject { task } context 'with same id' do - it { is_expected.to eq(described_class.new(job_payload.merge(id: task_id))) } + it { is_expected.to eq(described_class.new(**job_payload.merge(id: task_id))) } end context 'with different id' do - it { is_expected.not_to eq(described_class.new(job_payload.merge(id: task_id + 'a'))) } + it { is_expected.not_to eq(described_class.new(**job_payload.merge(id: task_id + 'a'))) } end context 'with different object' do diff --git a/spec/cloudtasker/cloud_task_spec.rb b/spec/cloudtasker/cloud_task_spec.rb index 63c1f55d..efab4264 100644 --- a/spec/cloudtasker/cloud_task_spec.rb +++ b/spec/cloudtasker/cloud_task_spec.rb @@ -46,7 +46,7 @@ before { allow(backend).to receive(:find).with(id).and_return(call_resp) } context 'with response' do - it { is_expected.to eq(described_class.new(payload)) } + it { is_expected.to eq(described_class.new(**payload)) } end context 'with no response' do @@ -65,7 +65,7 @@ before { allow(backend).to receive(:create).with(payload).and_return(call_resp) } context 'with response' do - it { is_expected.to eq(described_class.new(payload)) } + it { is_expected.to eq(described_class.new(**payload)) } end context 'with no response' do @@ -93,20 +93,20 @@ end describe '.new' do - subject { described_class.new(payload) } + subject { described_class.new(**payload) } it { is_expected.to have_attributes(payload) } end describe '#==' do - subject { described_class.new(payload) } + subject { described_class.new(**payload) } context 'with same id' do - it { is_expected.to eq(described_class.new(payload)) } + it { is_expected.to eq(described_class.new(**payload)) } end context 'with different id' do - it { is_expected.not_to eq(described_class.new(payload.merge(id: payload[:id] + 'a'))) } + it { is_expected.not_to eq(described_class.new(**payload.merge(id: payload[:id] + 'a'))) } end context 'with different object' do diff --git a/spec/cloudtasker/cron/schedule_spec.rb b/spec/cloudtasker/cron/schedule_spec.rb index 6227eff0..c824b6f6 100644 --- a/spec/cloudtasker/cron/schedule_spec.rb +++ b/spec/cloudtasker/cron/schedule_spec.rb @@ -149,7 +149,7 @@ end describe '.new' do - subject { described_class.new(attrs) } + subject { described_class.new(**attrs) } let(:attrs) do { diff --git a/spec/cloudtasker/worker_spec.rb b/spec/cloudtasker/worker_spec.rb index 0332f921..fe39a927 100644 --- a/spec/cloudtasker/worker_spec.rb +++ b/spec/cloudtasker/worker_spec.rb @@ -118,7 +118,7 @@ end describe '.schedule' do - subject { worker_class.schedule(opts) } + subject { worker_class.schedule(**opts) } let(:queue) { 'some-queue' } let(:delay) { 10 } @@ -178,7 +178,7 @@ end describe '.new' do - subject { worker_class.new(worker_args) } + subject { worker_class.new(**worker_args) } let(:task_id) { SecureRandom.uuid } let(:id) { SecureRandom.uuid } diff --git a/spec/cloudtasker/worker_wrapper_spec.rb b/spec/cloudtasker/worker_wrapper_spec.rb index ad4d9c43..d7e7dbbd 100644 --- a/spec/cloudtasker/worker_wrapper_spec.rb +++ b/spec/cloudtasker/worker_wrapper_spec.rb @@ -12,7 +12,7 @@ end describe '.new' do - subject { described_class.new(worker_args.merge(worker_name: worker_class)) } + subject { described_class.new(**worker_args.merge(worker_name: worker_class)) } let(:id) { SecureRandom.uuid } let(:args) { [1, 2] } @@ -47,7 +47,7 @@ let(:job_meta) { { foo: 'bar' } } let(:job_queue) { 'critical' } let(:attrs) { { worker_name: worker_class, job_queue: job_queue, job_args: job_args, job_meta: job_meta } } - let(:worker) { described_class.new(attrs) } + let(:worker) { described_class.new(**attrs) } it { is_expected.to have_attributes(attrs.merge(job_meta: eq(job_meta))) } it { expect(new_instance.job_id).not_to eq(worker.job_id) } diff --git a/spec/support/test_middleware.rb b/spec/support/test_middleware.rb index 5db0175b..f941d125 100644 --- a/spec/support/test_middleware.rb +++ b/spec/support/test_middleware.rb @@ -7,10 +7,10 @@ def initialize(arg = nil) @arg = arg end - def call(worker, **kwargs) + def call(worker, opts = {}) @called = true worker.middleware_called = true if worker.respond_to?(:middleware_called) - worker.middleware_opts = kwargs if worker.respond_to?(:middleware_opts) + worker.middleware_opts = opts if worker.respond_to?(:middleware_opts) yield end end