From 98abcf87d2c112e0a51e5fb1fd12689e3693e528 Mon Sep 17 00:00:00 2001 From: James Hong Date: Thu, 14 Aug 2025 15:39:00 -0700 Subject: [PATCH 01/26] [NPE-691] Drop support for Ruby 2.7 --- .github/workflows/unit_test-pr.yaml | 1 - Gemfile.ruby27 | 4 -- Gemfile.ruby27.lock | 61 ----------------------------- 3 files changed, 66 deletions(-) delete mode 100644 Gemfile.ruby27 delete mode 100644 Gemfile.ruby27.lock diff --git a/.github/workflows/unit_test-pr.yaml b/.github/workflows/unit_test-pr.yaml index aa8d25d..1e9c558 100644 --- a/.github/workflows/unit_test-pr.yaml +++ b/.github/workflows/unit_test-pr.yaml @@ -11,7 +11,6 @@ jobs: matrix: dependency_file: - Gemfile.latest - - Gemfile.ruby27 - Gemfile.ruby31 - Gemfile.ruby32 uses: ./.github/workflows/unit_test-run.yaml diff --git a/Gemfile.ruby27 b/Gemfile.ruby27 deleted file mode 100644 index 0847268..0000000 --- a/Gemfile.ruby27 +++ /dev/null @@ -1,4 +0,0 @@ -source 'https://rubygems.org' -ruby '2.7.6' - -gemspec name: 'chore-core' diff --git a/Gemfile.ruby27.lock b/Gemfile.ruby27.lock deleted file mode 100644 index 6bb4bff..0000000 --- a/Gemfile.ruby27.lock +++ /dev/null @@ -1,61 +0,0 @@ -PATH - remote: . - specs: - chore-core (4.7.0) - aws-sdk-sqs (>= 1) - get_process_mem (>= 0.2.0) - multi_json - -GEM - remote: https://rubygems.org/ - specs: - aws-eventstream (1.3.0) - aws-partitions (1.983.0) - aws-sdk-core (3.209.1) - aws-eventstream (~> 1, >= 1.3.0) - aws-partitions (~> 1, >= 1.651.0) - aws-sigv4 (~> 1.9) - jmespath (~> 1, >= 1.6.1) - aws-sdk-sqs (1.86.0) - aws-sdk-core (~> 3, >= 3.207.0) - aws-sigv4 (~> 1.5) - aws-sigv4 (1.10.0) - aws-eventstream (~> 1, >= 1.0.2) - bigdecimal (3.1.8) - dalli (3.2.8) - diff-lcs (1.5.1) - ffi (1.17.0) - get_process_mem (1.0.0) - bigdecimal (>= 2.0) - ffi (~> 1.0) - jmespath (1.6.2) - multi_json (1.15.0) - rspec (3.13.0) - rspec-core (~> 3.13.0) - rspec-expectations (~> 3.13.0) - rspec-mocks (~> 3.13.0) - rspec-core (3.13.1) - rspec-support (~> 3.13.0) - rspec-expectations (3.13.3) - diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.13.0) - rspec-mocks (3.13.2) - diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.13.0) - rspec-support (3.13.1) - timecop (0.9.10) - -PLATFORMS - ruby - -DEPENDENCIES - chore-core! - dalli (>= 2) - rspec (>= 3) - timecop - -RUBY VERSION - ruby 2.7.6p219 - -BUNDLED WITH - 2.4.22 From 38f50eee344eb9a9916def048568e9d5e8fa44bf Mon Sep 17 00:00:00 2001 From: James Hong Date: Thu, 14 Aug 2025 15:39:27 -0700 Subject: [PATCH 02/26] [NPE-691] Move AWS to dev-dep --- chore-core.gemspec | 3 ++- chore.gemspec | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/chore-core.gemspec b/chore-core.gemspec index 9f36fee..98f1d9e 100644 --- a/chore-core.gemspec +++ b/chore-core.gemspec @@ -36,9 +36,10 @@ Gem::Specification.new do |s| s.summary = "Job processing... for the future!" s.add_runtime_dependency(%q, [">= 0"]) - s.add_runtime_dependency(%q, [">= 1"]) s.add_runtime_dependency('get_process_mem', [">= 0.2.0"]) s.add_development_dependency(%q, [">= 3"]) + s.add_development_dependency(%q, [">= 1"]) + s.add_development_dependency(%q, ["~> 2.23"]) s.add_development_dependency(%q, [">= 2"]) s.add_development_dependency(%q, [">= 0"]) end diff --git a/chore.gemspec b/chore.gemspec index 4378a97..98215d5 100644 --- a/chore.gemspec +++ b/chore.gemspec @@ -36,10 +36,11 @@ Gem::Specification.new do |s| s.summary = "Job processing... for the future!" s.add_runtime_dependency(%q, [">= 0"]) - s.add_runtime_dependency(%q, [">= 1"]) s.add_runtime_dependency(%q, [">= 0"]) s.add_runtime_dependency(%q, [">= 0"]) s.add_development_dependency(%q, [">= 3"]) + s.add_development_dependency(%q, [">= 1"]) + s.add_development_dependency(%q, ["~> 2.23"]) s.add_development_dependency(%q, [">= 2"]) s.add_development_dependency(%q, [">= 0"]) end From 8bba6403c935fea6fa08f50e3f97bc0c960d5e01 Mon Sep 17 00:00:00 2001 From: James Hong Date: Thu, 14 Aug 2025 15:40:24 -0700 Subject: [PATCH 03/26] [NPE-691] Add PubSub client --- lib/chore/queues/pubsub.rb | 119 +++++++++++++++++++++++++++++++ spec/chore/queues/pubsub_spec.rb | 71 ++++++++++++++++++ 2 files changed, 190 insertions(+) create mode 100644 lib/chore/queues/pubsub.rb create mode 100644 spec/chore/queues/pubsub_spec.rb diff --git a/lib/chore/queues/pubsub.rb b/lib/chore/queues/pubsub.rb new file mode 100644 index 0000000..cc13eb6 --- /dev/null +++ b/lib/chore/queues/pubsub.rb @@ -0,0 +1,119 @@ +module Chore + module Queues + module PubSub + class << self + attr_accessor :project_id, :credentials + + # Configure PubSub settings with a block + def configure + yield self if block_given? + end + end + + # Creates a configured PubSub client with the given options + # Creates a GCP Pub/Sub client using global configuration + def self.pubsub_client + require 'google/cloud/pubsub' + + # Verify compatible version + begin + gem_version = Gem.loaded_specs['google-cloud-pubsub']&.version + if gem_version && gem_version < Gem::Version.new('2.23.0') + raise "google-cloud-pubsub version #{gem_version} is not supported. Please use version ~> 2.23" + end + rescue => e + Chore.logger.warn "Could not verify google-cloud-pubsub version: #{e.message}" if defined?(Chore.logger) + end + + if self.project_id && self.credentials + Google::Cloud::PubSub.new( + project_id: self.project_id, + credentials: self.credentials + ) + else + Google::Cloud::PubSub.new + end + end + # Helper method to create topics and subscriptions based on the currently known list as provided by your configured Chore::Jobs + # This is meant to be invoked from a rake task, and not directly. + # These topics and subscriptions will be created with the default settings, which may not be ideal. + # This is meant only as a convenience helper for testing, and not as a way to create production quality topics/subscriptions in Pub/Sub + # + # @param [TrueClass, FalseClass] halt_on_existing Raise an exception if the topic already exists + # + # @return [Array] + def self.create_queues!(halt_on_existing=false) + raise RuntimeError.new('You must have at least one Chore Job before attempting to create Pub/Sub topics') unless Chore.prefixed_queue_names.length > 0 + + if halt_on_existing + existing = self.existing_queues + if existing.size > 0 + raise <<-ERROR.gsub(/^\s+/, '') + We found topics/subscriptions that already exist! Verify your queue names or prefix are setup correctly. + + The following queue names were found: + #{existing.join("\n")} + ERROR + end + end + + Chore.prefixed_queue_names.each do |queue_name| + Chore.logger.info "Chore Creating Pub/Sub Topic and Subscription: #{queue_name}" + topic = pubsub_client.create_topic(queue_name) + subscription_name = "#{queue_name}-sub" + + begin + topic.create_subscription(subscription_name) + rescue Google::Cloud::AlreadyExistsError + Chore.logger.info "Subscription #{subscription_name} already exists" + end + end + + Chore.prefixed_queue_names + end + + # Helper method to delete all known topics and subscriptions based on the list as provided by your configured Chore::Jobs + # This is meant to be invoked from a rake task, and not directly. + # + # @return [Array] + def self.delete_queues! + raise RuntimeError.new('You must have at least one Chore Job before attempting to delete Pub/Sub topics') unless Chore.prefixed_queue_names.length > 0 + + Chore.prefixed_queue_names.each do |queue_name| + begin + Chore.logger.info "Chore Deleting Pub/Sub Topic and Subscription: #{queue_name}" + subscription_name = "#{queue_name}-sub" + + # Delete subscription first + subscription = pubsub_client.subscription(subscription_name) + subscription.delete if subscription.exists? + + # Then delete topic + topic = pubsub_client.topic(queue_name) + topic.delete if topic.exists? + rescue => e + # This could fail for a few reasons - log out why it failed, then continue on + Chore.logger.error "Deleting Topic/Subscription: #{queue_name} failed because #{e}" + end + end + + Chore.prefixed_queue_names + end + + # Collect a list of topics/subscriptions that already exist + # + # @return [Array] + def self.existing_queues + Chore.prefixed_queue_names.select do |queue_name| + begin + topic = pubsub_client.topic(queue_name) + subscription = pubsub_client.subscription("#{queue_name}-sub") + topic.exists? || subscription.exists? + rescue => e + false + end + end + end + end + end +end \ No newline at end of file diff --git a/spec/chore/queues/pubsub_spec.rb b/spec/chore/queues/pubsub_spec.rb new file mode 100644 index 0000000..da0812e --- /dev/null +++ b/spec/chore/queues/pubsub_spec.rb @@ -0,0 +1,71 @@ +require 'spec_helper' + +describe Chore::Queues::PubSub do + include_context 'fake pubsub objects' + + before(:each) do + allow(Chore::Queues::PubSub).to receive(:pubsub_client).and_return(pubsub_client) + allow(Chore).to receive(:prefixed_queue_names).and_return([queue_name]) + end + + context "when managing queues" do + it 'should create topics and subscriptions that are defined in its internal job name list' do + expect(pubsub_client).to receive(:create_topic).with(queue_name) + expect(topic).to receive(:create_subscription).with(subscription_name) + Chore::Queues::PubSub.create_queues! + end + + it 'should delete topics and subscriptions that are defined in its internal job name list' do + expect(pubsub_client).to receive(:subscription).with(subscription_name).and_return(subscription) + expect(pubsub_client).to receive(:topic).with(queue_name).and_return(topic) + expect(subscription).to receive(:delete) + expect(topic).to receive(:delete) + Chore::Queues::PubSub.delete_queues! + end + + context 'and checking for existing queues' do + it 'checks for existing topics/subscriptions' do + expect(described_class).to receive(:existing_queues).and_return([]) + Chore::Queues::PubSub.create_queues!(true) + end + + it 'raises an error if a topic/subscription does exist' do + allow(described_class).to receive(:existing_queues).and_return([queue_name]) + expect{Chore::Queues::PubSub.create_queues!(true)}.to raise_error(RuntimeError) + end + + it 'does not raise an error if a topic/subscription does not exist' do + allow(described_class).to receive(:existing_queues).and_return([]) + expect{Chore::Queues::PubSub.create_queues!(true)}.not_to raise_error + end + end + + context 'when handling subscription already exists error' do + it 'should continue when subscription already exists' do + allow(topic).to receive(:create_subscription).and_raise(Google::Cloud::AlreadyExistsError.new('Already exists')) + expect(Chore.logger).to receive(:info).with("Chore Creating Pub/Sub Topic and Subscription: #{queue_name}") + expect(Chore.logger).to receive(:info).with("Subscription #{subscription_name} already exists") + expect { Chore::Queues::PubSub.create_queues! }.not_to raise_error + end + end + end + + describe '.existing_queues' do + it 'returns queues that exist' do + allow(topic).to receive(:exists?).and_return(true) + allow(subscription).to receive(:exists?).and_return(true) + expect(described_class.existing_queues).to eq([queue_name]) + end + + it 'filters out queues that do not exist' do + allow(topic).to receive(:exists?).and_return(false) + allow(subscription).to receive(:exists?).and_return(false) + expect(described_class.existing_queues).to eq([]) + end + + it 'handles errors gracefully' do + allow(pubsub_client).to receive(:topic).and_raise(StandardError.new('Connection error')) + expect(described_class.existing_queues).to eq([]) + end + end +end \ No newline at end of file From 02ab432ce01cb5adb88c0bd548e98a2de51f9406 Mon Sep 17 00:00:00 2001 From: James Hong Date: Thu, 14 Aug 2025 15:40:43 -0700 Subject: [PATCH 04/26] [NPE-691] Add Consumer/Publisher for PubSub --- lib/chore/queues/pubsub/consumer.rb | 172 ++++++++++++ lib/chore/queues/pubsub/publisher.rb | 62 +++++ spec/chore/queues/pubsub/consumer_spec.rb | 298 +++++++++++++++++++++ spec/chore/queues/pubsub/publisher_spec.rb | 91 +++++++ spec/support/queues/pubsub/fake_objects.rb | 51 ++++ 5 files changed, 674 insertions(+) create mode 100644 lib/chore/queues/pubsub/consumer.rb create mode 100644 lib/chore/queues/pubsub/publisher.rb create mode 100644 spec/chore/queues/pubsub/consumer_spec.rb create mode 100644 spec/chore/queues/pubsub/publisher_spec.rb create mode 100644 spec/support/queues/pubsub/fake_objects.rb diff --git a/lib/chore/queues/pubsub/consumer.rb b/lib/chore/queues/pubsub/consumer.rb new file mode 100644 index 0000000..2b4445c --- /dev/null +++ b/lib/chore/queues/pubsub/consumer.rb @@ -0,0 +1,172 @@ +require 'google/cloud/pubsub' +require 'chore/duplicate_detector' + +module Chore + module Queues + module PubSub + # GCP Pub/Sub Consumer for Chore. Requests messages from GCP Pub/Sub and passes them to be worked on. + # Also controls acknowledging completed messages within GCP Pub/Sub. + class Consumer < Chore::Consumer + # Initialize the reset at on class load + @@reset_at = Time.now + + Chore::CLI.register_option 'gcp_project_id', '--gcp-project-id PROJECT_ID', 'GCP Project ID for Pub/Sub' do |project_id| + Chore::Queues::PubSub.project_id = project_id + end + + Chore::CLI.register_option 'gcp_credentials', '--gcp-credentials PATH', 'Path to GCP service account credentials JSON file' do |credentials| + Chore::Queues::PubSub.credentials = credentials + end + + # Resets the API client connection and provides @@reset_at so we know when the last time that was done + def self.reset_connection! + @@reset_at = Time.now + end + + # @param [String] queue_name Name of GCP Pub/Sub topic + # @param [Hash] opts Options + def initialize(queue_name, opts={}) + super(queue_name, opts) + @subscription_name = "#{queue_name}-sub" + end + + # Ensure that the consumer is capable of running + def verify_connection! + unless subscription.exists? + raise "Subscription #{@subscription_name} does not exist" + end + end + + # Begins requesting messages from GCP Pub/Sub, which will invoke the +&handler+ over each message + # + # @param [Block] &handler Message handler, used by the calling context (worker) to create & assigns a UnitOfWork + # + # @return [Array] + def consume(&handler) + while running? + begin + messages = handle_messages(&handler) + sleep(Chore.config.consumer_sleep_interval) if messages.empty? + rescue => e + Chore.logger.error { "PubSubConsumer#consume: #{e.inspect} #{e.backtrace * "\n"}" } + end + end + end + + # Unimplemented. Rejects the given message from GCP Pub/Sub. + # + # @param [String] message_id Unique ID of the Pub/Sub message + # + # @return nil + def reject(message_id) + # In Pub/Sub, we can simply not acknowledge the message and it will be redelivered + end + + # Acknowledges the given message from the GCP Pub/Sub subscription + # + # @param [String] message_id Unique ID of the Pub/Sub message + # @param [String] ack_id Acknowledgment ID of the Pub/Sub message + def complete(message_id, ack_id) + Chore.logger.debug "Completing (acknowledging): #{message_id}" + # Find the message by ack_id and acknowledge it + if msg = @current_messages&.find { |m| m.ack_id == ack_id } + msg.acknowledge! + end + end + + # Delays retry of a job by +backoff_calc+ seconds. + # In Pub/Sub, we modify the ack deadline to delay the message + # + # @param [UnitOfWork] item Item to be delayed + # @param [Block] backoff_calc Code that determines the backoff. + def delay(item, backoff_calc) + delay = backoff_calc.call(item) + Chore.logger.debug "Delaying #{item.id} by #{delay} seconds" + + # Find the message and modify its ack deadline + if msg = @current_messages&.find { |m| m.ack_id == item.receipt_handle } + msg.modify_ack_deadline!(delay) + end + + return delay + end + + private + + # Requests messages from GCP Pub/Sub, and invokes the provided +&block+ over each one. Afterwards, the :on_fetch + # hook will be invoked, per message + # + # @param [Block] &handler Message handler, passed along by #consume + # + # @return [Array] + def handle_messages(&block) + begin + verify_connection! + rescue => e + Chore.logger.error "There was a problem verifying the connection to the subscription: #{e.message}. Shutting down..." + raise Chore::TerribleMistake + end + + messages = subscription.pull(max: max_messages) + @current_messages = messages + received_timestamp = Time.now + + messages.each do |message| + unless duplicate_message?(message.message_id, @subscription_name, queue_timeout, received_timestamp) + # delivery_attempt is available but may be nil for older messages + attempt_count = (message.delivery_attempt || 1) - 1 + block.call(message.message_id, message.ack_id, queue_name, queue_timeout, message.data, attempt_count, received_timestamp) + end + Chore.run_hooks_for(:on_fetch, message.ack_id, message.data) + end + + messages + end + + # Retrieves the GCP Pub/Sub subscription object. The method will cache the results to prevent round trips on subsequent calls + # + # If reset_connection! has been called, this will result in the connection being re-initialized, + # as well as clear any cached results from prior calls + # + # @return [Google::Cloud::PubSub::Subscription] + def subscription + if !@pubsub_last_connected || (@@reset_at && @@reset_at >= @pubsub_last_connected) + @pubsub = nil + @pubsub_last_connected = Time.now + @subscription = nil + end + + @subscription ||= pubsub.subscription(@subscription_name) + end + + # The ack deadline (in seconds) of the subscription + # + # @return [Integer] + def queue_timeout + @queue_timeout ||= subscription.ack_deadline_seconds || 600 # Default to 10 minutes + end + + # GCP Pub/Sub client object + # + # @return [Google::Cloud::PubSub::Project] + def pubsub + @pubsub ||= Chore::Queues::PubSub.pubsub_client + end + + # Maximum number of messages to retrieve on each request + # + # @return [Integer] + def max_messages + [pubsub_polling_amount, 1000].min # Pub/Sub max is 1000 + end + + # Maximum number of messages to retrieve on each request from config + # + # @return [Integer] + def pubsub_polling_amount + Chore.config.queue_polling_size + end + end + end + end +end diff --git a/lib/chore/queues/pubsub/publisher.rb b/lib/chore/queues/pubsub/publisher.rb new file mode 100644 index 0000000..dbd9856 --- /dev/null +++ b/lib/chore/queues/pubsub/publisher.rb @@ -0,0 +1,62 @@ +require 'chore/publisher' + +module Chore + module Queues + module PubSub + # GCP Pub/Sub Publisher, for writing messages to GCP Pub/Sub from Chore + class Publisher < Chore::Publisher + @@reset_next = true + + # @param [Hash] opts Publisher options + def initialize(opts={}) + super + @pubsub_topics = {} + end + + # Publishes a message to a GCP Pub/Sub topic + # + # @param [String] queue_name Name of the GCP Pub/Sub topic + # @param [Hash] job Job instance definition, will be encoded to JSON + # + # @return [Google::Cloud::PubSub::Message] + def publish(queue_name, job) + topic = get_topic(queue_name) + encoded_job = encode_job(job) + topic.publish(encoded_job) + end + + # Sets a flag that instructs the publisher to reset the connection the next time it's used + def self.reset_connection! + @@reset_next = true + end + + private + + # GCP Pub/Sub client object + # + # @return [Google::Cloud::PubSub::Project] + def pubsub + @pubsub ||= Chore::Queues::PubSub.pubsub_client + end + + # Retrieves the GCP Pub/Sub topic object. The method will cache the results to prevent round trips on subsequent calls + # + # If reset_connection! has been called, this will result in the connection being re-initialized, + # as well as clear any cached results from prior calls + # + # @param [String] name Name of GCP Pub/Sub topic + # + # @return [Google::Cloud::PubSub::Topic] + def get_topic(name) + if @@reset_next + @pubsub = nil + @@reset_next = false + @pubsub_topics = {} + end + + @pubsub_topics[name] ||= pubsub.topic(name) + end + end + end + end +end \ No newline at end of file diff --git a/spec/chore/queues/pubsub/consumer_spec.rb b/spec/chore/queues/pubsub/consumer_spec.rb new file mode 100644 index 0000000..4e1dbfb --- /dev/null +++ b/spec/chore/queues/pubsub/consumer_spec.rb @@ -0,0 +1,298 @@ +require 'spec_helper' + +describe Chore::Queues::PubSub::Consumer do + include_context 'fake pubsub objects' + + let(:options) { {} } + let(:consumer) { Chore::Queues::PubSub::Consumer.new(queue_name) } + let(:job) { {'class' => 'TestJob', 'args'=>[1,2,'3']} } + let(:backoff_func) { Proc.new { 2 + 2 } } + + # Since a message handler is required (but not validated), this convenience method lets us + # effectively stub the block. + def consume(&block) + block = Proc.new{} unless block_given? + consumer.consume(&block) + end + + before do + allow(Chore::Queues::PubSub).to receive(:pubsub_client).and_return(pubsub_client) + allow(subscription).to receive(:pull).and_return([received_message]) + end + + describe "consuming messages" do + before do + allow(consumer).to receive(:running?).and_return(true, false) + end + + context "should create objects for interacting with the Pub/Sub API" do + it 'should create a pubsub client' do + expect(subscription).to receive(:pull) + consume + end + + it "should only create a pubsub client when one doesn't exist" do + allow(consumer).to receive(:running?).and_return(true, true, true, true, false, true, true) + expect(Chore::Queues::PubSub).to receive(:pubsub_client).exactly(:once) + consume + end + + it 'should look up the subscription based on the queue name' do + expect(pubsub_client).to receive(:subscription).with(subscription_name) + consume + end + + it 'should create a subscription object' do + expect(consumer.send(:subscription)).to_not be_nil + consume + end + end + + context "should receive a message from the subscription" do + it 'should use the default size of 10 when no queue_polling_size is specified' do + expect(subscription).to receive(:pull).with(max: 10).and_return([received_message]) + consume + end + + it 'should respect the queue_polling_size when specified' do + allow(Chore.config).to receive(:queue_polling_size).and_return(5) + expect(subscription).to receive(:pull).with(max: 5) + consume + end + + it 'should respect Pub/Sub max limit of 1000 messages' do + allow(Chore.config).to receive(:queue_polling_size).and_return(2000) + expect(subscription).to receive(:pull).with(max: 1000) + consume + end + end + + context 'with no messages' do + before do + allow(consumer).to receive(:handle_messages).and_return([]) + end + + it 'should sleep' do + expect(consumer).to receive(:sleep).with(1) + consume + end + end + + context 'with messages' do + before do + allow(consumer).to receive(:duplicate_message?).and_return(false) + allow(subscription).to receive(:pull).and_return([received_message]) + allow(Time).to receive(:now).and_return(Time.utc(2024, 5, 10, 12, 0, 0)) + end + + it "should check the uniqueness of the message" do + expect(consumer).to receive(:duplicate_message?) + consume + end + + let(:received_timestamp) { Time.utc(2024, 5, 10, 12, 0, 0) } + + it "should yield the message to the handler block" do + expect { |b| consume(&b) } + .to yield_with_args( + received_message.message_id, + received_message.ack_id, + queue_name, + subscription.ack_deadline_seconds, + received_message.data, + 0, # delivery_attempt - 1 (1 - 1 = 0) + received_timestamp + ) + end + + it 'should not sleep' do + expect(consumer).to_not receive(:sleep) + consume + end + + context 'with duplicates' do + before do + allow(consumer).to receive(:duplicate_message?).and_return(true) + end + + it 'should not yield for a dupe message' do + expect {|b| consume(&b) }.not_to yield_control + end + end + + context 'with delivery attempt count' do + before do + allow(received_message).to receive(:delivery_attempt).and_return(3) + end + + it 'should calculate attempt count as delivery_attempt - 1' do + expect { |b| consume(&b) } + .to yield_with_args( + received_message.message_id, + received_message.ack_id, + queue_name, + subscription.ack_deadline_seconds, + received_message.data, + 2, # delivery_attempt - 1 (3 - 1 = 2) + received_timestamp + ) + end + end + + context 'with nil delivery attempt (older messages)' do + before do + allow(received_message).to receive(:delivery_attempt).and_return(nil) + end + + it 'should default to attempt count of 0' do + expect { |b| consume(&b) } + .to yield_with_args( + received_message.message_id, + received_message.ack_id, + queue_name, + subscription.ack_deadline_seconds, + received_message.data, + 0, # (nil || 1) - 1 = 0 + received_timestamp + ) + end + end + end + + context 'on subscription lookup failure' do + before(:each) do + allow(consumer).to receive(:verify_connection!).and_raise(Google::Cloud::NotFoundError.new('Subscription not found')) + end + + it 'should raise exception' do + expect { consume }.to raise_error(Chore::TerribleMistake) + end + end + + context 'on gcp credential failure' do + before(:each) do + allow(consumer).to receive(:verify_connection!).and_raise(Google::Cloud::PermissionDeniedError.new('Permission denied')) + end + + it 'should raise exception' do + expect { consume }.to raise_error(Chore::TerribleMistake) + end + end + + context 'on unexpected failure' do + before(:each) do + allow(consumer).to receive(:verify_connection!).and_raise(StandardError.new('Connection error')) + end + + it 'should raise exception' do + expect { consume }.to raise_error(Chore::TerribleMistake) + end + end + end + + describe "completing work" do + before do + # Set up current messages for the complete method to work + consumer.instance_variable_set(:@current_messages, [received_message]) + end + + it 'acknowledges the message in the subscription' do + expect(received_message).to receive(:acknowledge!) + consumer.complete(received_message.message_id, received_message.ack_id) + end + + it 'handles missing message gracefully' do + expect { consumer.complete('unknown-id', 'unknown-ack') }.not_to raise_error + end + end + + describe '#delay' do + let(:item) { Chore::UnitOfWork.new(received_message.message_id, received_message.ack_id, queue_name, 600, received_message.data, 0, consumer) } + + before do + # Set up current messages for the delay method to work + consumer.instance_variable_set(:@current_messages, [received_message]) + end + + it 'modifies the ack deadline of the message' do + delay_seconds = backoff_func.call(item) + expect(received_message).to receive(:modify_ack_deadline!).with(delay_seconds) + consumer.delay(item, backoff_func) + end + + it 'returns the delay value' do + delay_seconds = backoff_func.call(item) + result = consumer.delay(item, backoff_func) + expect(result).to eq(delay_seconds) + end + + it 'handles missing message gracefully' do + unknown_item = Chore::UnitOfWork.new('unknown-id', 'unknown-ack', queue_name, 600, 'data', 0, consumer) + expect { consumer.delay(unknown_item, backoff_func) }.not_to raise_error + end + end + + describe '#reject' do + it 'should be implemented but do nothing (Pub/Sub handles redelivery automatically)' do + expect { consumer.reject('message-id') }.not_to raise_error + end + end + + describe '#reset_connection!' do + it 'should reset the connection after a call to reset_connection!' do + # First call establishes the connection + consumer.send(:subscription) + + # Reset connection + Chore::Queues::PubSub::Consumer.reset_connection! + + # Should recreate client + expect(Chore::Queues::PubSub).to receive(:pubsub_client).and_return(pubsub_client) + consumer.send(:subscription) + end + + it 'should not reset the connection between calls' do + expect(Chore::Queues::PubSub).to receive(:pubsub_client).once.and_return(pubsub_client) + s = consumer.send(:subscription) + expect(consumer.send(:subscription)).to be(s) + end + + it 'should reconfigure pubsub client' do + allow(consumer).to receive(:running?).and_return(true, false) + allow_any_instance_of(Chore::DuplicateDetector).to receive(:found_duplicate?).and_return(false) + + allow(subscription).to receive(:pull).and_return([received_message]) + + consume + + Chore::Queues::PubSub::Consumer.reset_connection! + allow(Chore::Queues::PubSub).to receive(:pubsub_client).and_return(pubsub_client) + + expect(consumer).to receive(:running?).and_return(true, false) + consume + end + end + + describe '#verify_connection!' do + it 'should verify subscription exists' do + expect(subscription).to receive(:exists?).and_return(true) + expect { consumer.verify_connection! }.not_to raise_error + end + + it 'should raise error if subscription does not exist' do + allow(subscription).to receive(:exists?).and_return(false) + expect { consumer.verify_connection! }.to raise_error + end + end + + describe 'queue timeout' do + it 'should return subscription ack deadline' do + expect(consumer.send(:queue_timeout)).to eq(600) + end + + it 'should default to 600 seconds if ack_deadline_seconds is nil' do + allow(subscription).to receive(:ack_deadline_seconds).and_return(nil) + expect(consumer.send(:queue_timeout)).to eq(600) + end + end +end \ No newline at end of file diff --git a/spec/chore/queues/pubsub/publisher_spec.rb b/spec/chore/queues/pubsub/publisher_spec.rb new file mode 100644 index 0000000..b7b3c93 --- /dev/null +++ b/spec/chore/queues/pubsub/publisher_spec.rb @@ -0,0 +1,91 @@ +require 'spec_helper' + +describe Chore::Queues::PubSub::Publisher do + include_context 'fake pubsub objects' + + let(:publisher) { Chore::Queues::PubSub::Publisher.new } + let(:job) { {'class' => 'TestJob', 'args'=>[1,2,'3']} } + let(:publish_result) { double('Google::Cloud::PubSub::Message', message_id: message_id) } + + before(:each) do + allow(Chore::Queues::PubSub).to receive(:pubsub_client).and_return(pubsub_client) + allow(topic).to receive(:publish).and_return(publish_result) + end + + it 'should configure pubsub client' do + expect(Chore::Queues::PubSub).to receive(:pubsub_client) + publisher.publish(queue_name, job) + end + + it 'should not create a new Pub/Sub client before every publish' do + expect(Chore::Queues::PubSub).to receive(:pubsub_client).once + 2.times { publisher.send(:get_topic, queue_name) } + end + + it 'should lookup the topic when publishing' do + expect(pubsub_client).to receive(:topic).with(queue_name).and_return(topic) + publisher.publish(queue_name, job) + end + + it 'should publish an encoded message to the specified topic' do + expect(topic).to receive(:publish).with(job.to_json) + publisher.publish(queue_name, job) + end + + it 'should lookup multiple topics if specified' do + second_queue_name = queue_name + '2' + second_topic = double('Google::Cloud::PubSub::Topic') + + expect(pubsub_client).to receive(:topic).with(queue_name).and_return(topic) + expect(pubsub_client).to receive(:topic).with(second_queue_name).and_return(second_topic) + expect(topic).to receive(:publish) + expect(second_topic).to receive(:publish) + + publisher.publish(queue_name, job) + publisher.publish(second_queue_name, job) + end + + it 'should only lookup a named topic once' do + expect(pubsub_client).to receive(:topic).with(queue_name).once.and_return(topic) + expect(topic).to receive(:publish).exactly(4).times + 4.times { publisher.publish(queue_name, job) } + end + + describe '#reset_connection!' do + it 'should reset client connection after a call to reset_connection!' do + # First call establishes the client + publisher.send(:get_topic, queue_name) + + # Reset and verify client is reset + Chore::Queues::PubSub::Publisher.reset_connection! + + expect(Chore::Queues::PubSub).to receive(:pubsub_client).and_return(pubsub_client) + publisher.send(:get_topic, queue_name) + end + + it 'should clear topic cache after reset_connection!' do + # Establish topic cache + publisher.send(:get_topic, queue_name) + + # Reset connection + Chore::Queues::PubSub::Publisher.reset_connection! + + # Should lookup topic again + expect(pubsub_client).to receive(:topic).with(queue_name).and_return(topic) + publisher.send(:get_topic, queue_name) + end + + it 'should not reset the connection between calls' do + expect(Chore::Queues::PubSub).to receive(:pubsub_client).once.and_return(pubsub_client) + Chore::Queues::PubSub::Publisher.reset_connection! + 4.times { publisher.send(:get_topic, queue_name) } + end + end + + describe 'encoding' do + it 'should encode job to JSON' do + encoded_job = publisher.send(:encode_job, job) + expect(encoded_job).to eq(job.to_json) + end + end +end \ No newline at end of file diff --git a/spec/support/queues/pubsub/fake_objects.rb b/spec/support/queues/pubsub/fake_objects.rb new file mode 100644 index 0000000..df0a2b4 --- /dev/null +++ b/spec/support/queues/pubsub/fake_objects.rb @@ -0,0 +1,51 @@ +describe Chore::Queues::PubSub do + RSpec.shared_context 'fake pubsub objects' do + let(:queue_name) { 'test_queue' } + let(:subscription_name) { "#{queue_name}-sub" } + let(:project_id) { 'test-project' } + + let(:message_data) { {'class' => 'TestJob', 'args' => [1, 2, '3']}.to_json } + let(:message_id) { 'message-id-123' } + let(:ack_id) { 'ack-id-456' } + + let(:received_message) do + double('Google::Cloud::PubSub::ReceivedMessage', + message_id: message_id, + ack_id: ack_id, + data: message_data, + delivery_attempt: 1, + acknowledge!: true, + modify_ack_deadline!: true + ) + end + + let(:topic) do + double('Google::Cloud::PubSub::Topic', + name: queue_name, + exists?: true, + publish: received_message, + create_subscription: subscription, + delete: true + ) + end + + let(:subscription) do + double('Google::Cloud::PubSub::Subscription', + name: subscription_name, + exists?: true, + ack_deadline_seconds: 600, + pull: [received_message], + delete: true + ) + end + + let(:pubsub_client) do + double('Google::Cloud::PubSub::Project', + project_id: project_id, + topic: topic, + subscription: subscription, + create_topic: topic + ) + end + end +end \ No newline at end of file From 5317c7a03173c027690a7451cbe43669180ee49b Mon Sep 17 00:00:00 2001 From: James Hong Date: Thu, 14 Aug 2025 15:41:06 -0700 Subject: [PATCH 05/26] [NPE-691] Update queue creation tasks --- lib/chore/queues/sqs.rb | 8 +++---- lib/chore/tasks/queues.task | 48 +++++++++++++++++++++++++++++-------- 2 files changed, 42 insertions(+), 14 deletions(-) diff --git a/lib/chore/queues/sqs.rb b/lib/chore/queues/sqs.rb index 4b7413c..2ec55e8 100644 --- a/lib/chore/queues/sqs.rb +++ b/lib/chore/queues/sqs.rb @@ -19,7 +19,7 @@ def self.sqs_client # # @return [Array] def self.create_queues!(halt_on_existing=false) - raise 'You must have atleast one Chore Job configured and loaded before attempting to create queues' unless Chore.prefixed_queue_names.length > 0 + raise RuntimeError.new('You must have at least one Chore Job before attempting to create SQS queues') unless Chore.prefixed_queue_names.length > 0 if halt_on_existing existing = self.existing_queues @@ -34,7 +34,7 @@ def self.create_queues!(halt_on_existing=false) end Chore.prefixed_queue_names.each do |queue_name| - Chore.logger.info "Chore Creating Queue: #{queue_name}" + Chore.logger.info "Chore Creating SQS Queue: #{queue_name}" begin sqs_client.create_queue(queue_name: queue_name) rescue Aws::SQS::Errors::QueueAlreadyExists @@ -51,11 +51,11 @@ def self.create_queues!(halt_on_existing=false) # @return [Array] def self.delete_queues! - raise 'You must have atleast one Chore Job configured and loaded before attempting to create queues' unless Chore.prefixed_queue_names.length > 0 + raise RuntimeError.new('You must have at least one Chore Job before attempting to delete SQS queues') unless Chore.prefixed_queue_names.length > 0 Chore.prefixed_queue_names.each do |queue_name| begin - Chore.logger.info "Chore Deleting Queue: #{queue_name}" + Chore.logger.info "Chore Deleting SQS Queue: #{queue_name}" url = sqs_client.get_queue_url(queue_name: queue_name).queue_url sqs_client.delete_queue(queue_url: url) rescue => e diff --git a/lib/chore/tasks/queues.task b/lib/chore/tasks/queues.task index c0e6ce6..c4d9623 100644 --- a/lib/chore/tasks/queues.task +++ b/lib/chore/tasks/queues.task @@ -1,18 +1,46 @@ namespace :chore do - desc <<-DESC.gsub(/^\s+/, '') - Create all defined queues. If the halt_on_existing argument is set (defaults to off) the task will abort if a single - queue already exists without attempting to create any. + namespace :queues do + desc <<-DESC.gsub(/^\s+/, '') + Create all defined SQS queues. + If the halt_on_existing argument is set (defaults to off) the task will abort if a single + queue already exists without attempting to create any. - This flag is specifically provided for our integration testing platform to ensure we don't deploy to an incorrect environment. - DESC - task :create, :halt_on_existing do |t, args| - halt_on_existing = %w(1 true yes t y).include?(args[:halt_on_existing]) + This flag is specifically provided for our integration testing platform to ensure we don't deploy to an incorrect environment. + DESC + task :create_sqs, :halt_on_existing do |t, args| + halt_on_existing = %w(1 true yes t y).include?(args[:halt_on_existing]) + created_queues = Chore::Queues::SQS.create_queues!(halt_on_existing) + Chore.logger.info "Successfully created #{created_queues.length} SQS queues: #{created_queues.join(', ')}" + end + + desc "Create all defined Pub/Sub topics and subscriptions" + task :create_pubsub, :halt_on_existing do |t, args| + halt_on_existing = %w(1 true yes t y).include?(args[:halt_on_existing]) + created_queues = Chore::Queues::PubSub.create_queues!(halt_on_existing) + Chore.logger.info "Successfully created #{created_queues.length} Pub/Sub topics: #{created_queues.join(', ')}" + end + + desc "Remove all defined SQS queues" + task :remove_sqs do + deleted_queues = Chore::Queues::SQS.delete_queues! + Chore.logger.info "Successfully deleted #{deleted_queues.length} SQS queues: #{deleted_queues.join(', ')}" + end - Chore::Queues::SQS.create_queues!(halt_on_existing) + desc "Remove all defined Pub/Sub topics and subscriptions" + task :remove_pubsub do + deleted_queues = Chore::Queues::PubSub.delete_queues! + Chore.logger.info "Successfully deleted #{deleted_queues.length} Pub/Sub topics: #{deleted_queues.join(', ')}" + end + end + + # Legacy tasks for backward compatibility + desc "Create all defined SQS queues (legacy - use chore:queues:create_sqs instead)" + task :create, :halt_on_existing do |t, args| + Rake::Task['chore:queues:create_sqs'].invoke(args[:halt_on_existing]) end - desc "Remove all defined queues" + desc "Remove all defined SQS queues (legacy - use chore:queues:remove_sqs instead)" task :remove do - Chore::Queues::SQS.delete_queues! + Rake::Task['chore:queues:remove_sqs'].invoke end end From c5efb663c59948aec140675cfb76eaed2a255017 Mon Sep 17 00:00:00 2001 From: James Hong Date: Thu, 14 Aug 2025 15:44:54 -0700 Subject: [PATCH 06/26] [NPE-691] Bump version/Update README/CHANGELOG --- CHANGELOG.md | 11 +++ README.md | 167 +++++++++++++++++++++++++++++++++++++++++-- lib/chore/version.rb | 4 +- 3 files changed, 175 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 047b65f..a8ff856 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,17 @@ ## [master](https://github.com/Tapjoy/chore/tree/master) +**Features** +- Google Pubsub support + +**Fixed bugs** +- N/A + +**Cleanups** +- Drops Ruby 2.7 support + +## [v4.7.5](https://github.com/Tapjoy/chore/tree/v4.7.5) + **Features** - N/A diff --git a/README.md b/README.md index d25c629..35f0b99 100644 --- a/README.md +++ b/README.md @@ -16,11 +16,24 @@ Chore can be integrated with any Ruby-based project by following these instructi 1. Add `chore-core` to the Gemfile ```ruby - gem 'chore-core', '~> 4.0.0' + gem 'chore-core', '~> 5.0.0' ``` - When using SQS, also add `dalli` to use for `memcached`-based deduplication: + **Queue Provider Dependencies**: Add the appropriate gem for your queue provider: + For **Amazon SQS**: + ```ruby + gem 'aws-sdk-sqs' + ``` + + For **Google Cloud Pub/Sub**: + ```ruby + gem 'google-cloud-pubsub', '~> 2.23' + ``` + + For **Filesystem queues**: No additional gems required. + + **Optional: Message Deduplication**: If you want to use memcached-based message deduplication: ```ruby gem 'dalli' ``` @@ -40,6 +53,59 @@ dependencies and required code. `AWS_SECRET_ACCESS_KEY` environment variables) and an AWS region is set (e.g. `AWS_REGION` environment variable) so that Chore can authenticate with AWS. + To use GCP Pub/Sub with Chore, you have several configuration options: + + **Option 1: Basic Setup** + Configure Chore to use the PubSub consumer and publisher: + + ```ruby + Chore.configure do |c| + c.consumer = Chore::Queues::PubSub::Consumer + c.publisher = Chore::Queues::PubSub::Publisher + end + ``` + + **Note**: Setting the consumer does not automatically set the publisher. You must configure both separately. + + **Note**: This configuration will use Google Cloud's automatic credential discovery (environment variables, service account, etc.). See the [Google Cloud PubSub Authentication documentation](https://cloud.google.com/ruby/docs/reference/google-cloud-pubsub/latest/AUTHENTICATION) for details. + + **Option 2: Global Configuration** + Set project and credentials that apply to all PubSub operations: + + ```ruby + Chore::Queues::PubSub.configure do |config| + config.project_id = 'your-project-id' + config.credentials = '/path/to/key.json' + end + ``` + + **Note**: This is equivalent to using the command line options `--gcp-project-id` and `--gcp-credentials`. See [Command Line Options](#command-line-options) for details. + + **Option 3: Per-Job Publisher Override** + Override the global publisher for specific jobs using `queue_options`: + + ```ruby + class MyJob + include Chore::Job + queue_options( + name: 'my_queue', + publisher: Chore::Queues::PubSub::Publisher + ) + + def perform(args={}) + # Job logic here + end + end + ``` + + **Note**: Publisher configuration (project_id, credentials) is set globally via environment variables or the `Chore::Queues::PubSub.configure` block. Individual jobs cannot override these settings. + + **Note**: PubSub requires gRPC which can have threading issues in some environments. If using PubSub in a threaded environment (such as with threaded consumer strategies), ensure you use gRPC version 1.74.1 or higher to avoid potential deadlocks and connection issues. + + **Warning**: Using different queue types for consumer and publisher (e.g., PubSub consumer with SQS publisher) can be confusing and is generally not recommended. Mixed configurations should only be used carefully for specific scenarios like migrations, validations, or no-op queues. + + **Configuration Priority**: Global configuration → Environment variables + By default, Chore will run over all queues it detects among the required files. If different behavior is desired, use one of the following flags: @@ -52,6 +118,25 @@ Chore can authenticate with AWS. 1. Chore has many more options, which can be viewed by executing `bundle exec chore --help` +### Command Line Options + +In addition to configuration blocks, Chore supports command line options for queue-specific settings: + +**AWS SQS Options:** +``` +--aws-access-key KEY Valid AWS Access Key +--aws-secret-key KEY Valid AWS Secret Key +--dedupe-servers SERVERS List of memcache compatible server(s) for SQS message deduplication +``` + +**GCP Pub/Sub Options:** +``` +--gcp-project-id PROJECT_ID GCP Project ID for Pub/Sub +--gcp-credentials PATH Path to GCP service account credentials JSON file +``` + +These command line options set global defaults and are equivalent to setting the corresponding configuration in your code. + ### Tips For Configuring Chore For Rails, it can be necessary to add the jobs directory to the eager loading path, found in `application.rb`. A similar @@ -97,6 +182,7 @@ Chore.configure do |c| end ``` + Because it is like that the same application serves as the basis for both producing and consuming messages, and there will already be a considerable amount of configuration in the Producer, it makes sense to use Chorefile to simply provide the `require` option and stick to the initializer for the rest of the configuration to keep things DRY. @@ -117,12 +203,27 @@ This section assumes `foreman` is being used to execute (or export the run comma 1. If the queues do not exist, they must be created before the application can produce/consume Chore jobs: + For SQS: ```ruby require 'aws-sdk-sqs' sqs = Aws::SQS::Client.new sqs.create_queue(queue_name: "test_queue") ``` + For GCP Pub/Sub: + ```ruby + require 'google/cloud/pubsub' + pubsub = Google::Cloud::PubSub.new + topic = pubsub.create_topic "test_queue" + topic.create_subscription "test_queue-sub" + ``` + + Alternatively, you can use Chore's built-in queue management: + ```ruby + # This will create both topics and subscriptions for Pub/Sub + Chore::Queues::PubSub.create_queues!(['test_queue']) + ``` + 1. Finally, start the application as usual ``` @@ -179,15 +280,57 @@ Chore.configure do |c| end ``` +Chore provides the following built-in publishers: + +* `Chore::Queues::SQS::Publisher` - For Amazon SQS +* `Chore::Queues::PubSub::Publisher` - For Google Cloud Pub/Sub +* `Chore::Queues::Filesystem::Publisher` - For filesystem-based queues + +**Note**: When using the `Chore::Queues::PubSub::Publisher`, you can configure it per-job, set defaults, or rely on Google Cloud's automatic credential discovery as described in the [GCP Pub/Sub Configuration Example](#gcp-pubsub-configuration-example) section. + It is worth noting that any option that can be set via config file or command-line args can also be set in a configure block. -If a global publisher is set, it can be overridden on a per-job basis by specifying the publisher in `queue_options`. +If a global publisher is set, it can be overridden on a per-job basis by specifying the publisher in `queue_options`: + +```ruby +class MyJob + include Chore::Job + queue_options :name => 'my_queue', :publisher => Some::Other::Publisher + + def perform(args={}) + # Job logic here + end +end +``` + +### Consumer Configuration + +Unlike publishers, **consumers are configured globally and cannot be set per-job**. Consumers operate at the process level, polling queues and dispatching work to workers. They handle all queues in the system. + +Consumers can be configured in three ways: + +1. **Command line**: `--consumer CLASS_NAME` +2. **Global configuration**: + ```ruby + Chore.configure do |c| + c.consumer = Your::Custom::Consumer + end + ``` +3. **Default**: `Chore::Queues::SQS::Consumer` if not specified + +Chore provides the following built-in consumers: + +* `Chore::Queues::SQS::Consumer` - For Amazon SQS (default) +* `Chore::Queues::PubSub::Consumer` - For Google Cloud Pub/Sub +* `Chore::Queues::Filesystem::Consumer` - For filesystem-based queues + +**Note**: Consumers cannot be specified in `queue_options` like publishers can. The consumer configuration applies to all queues processed by the Chore instance. This means each Chore instance can only use one consumer at a time. To use multiple consumers (e.g., both SQS and Pub/Sub), you would need to run separate Chore instances, each configured with its intended consumer. ## Retry Backoff Strategy -Chore has basic support for delaying retries of a failed job using a step function. Currently the only queue that -supports this functionality is SQS; all others will simply ignore the delay setting. +Chore has basic support for delaying retries of a failed job using a step function. Currently SQS and Pub/Sub +support this functionality; other queue types will simply ignore the delay setting. ### Setup @@ -207,6 +350,16 @@ If there is a `:backoff` option supplied, any failures will delay the next attem Read more details about SQS and Delays [here](docs/Delayed%20Jobs.md) +### Notes On GCP Pub/Sub + +GCP Pub/Sub uses a topic and subscription model. When using Chore with Pub/Sub: + +* Each queue corresponds to a Pub/Sub topic +* Subscriptions are automatically created with the naming pattern `{topic-name}-sub` +* Message delays are handled using `modify_ack_deadline!` instead of SQS visibility timeouts +* Messages are acknowledged using `acknowledge!` instead of being deleted +* Pull-based consumption is used with configurable batch sizes (up to 1000 messages) + ## Hooks A number of hooks, both global and per-job, exist in Chore for flexibility and convencience. Hooks should be named @@ -232,6 +385,10 @@ A number of hooks, both global and per-job, exist in Chore for flexibility and c * `on_fetch(handle, body)` +#### GCP Pub/Sub Consumer Hooks + +* `on_fetch(received_message, body)` + ### Per Job * `before_publish` diff --git a/lib/chore/version.rb b/lib/chore/version.rb index 71be4a0..8cab0d3 100644 --- a/lib/chore/version.rb +++ b/lib/chore/version.rb @@ -1,7 +1,7 @@ module Chore module Version #:nodoc: - MAJOR = 4 - MINOR = 7 + MAJOR = 5 + MINOR = 0 PATCH = 0 STRING = [ MAJOR, MINOR, PATCH ].join('.') From 46c98b912f2e531a95e4ad58954a4ed635713d51 Mon Sep 17 00:00:00 2001 From: James Hong Date: Thu, 14 Aug 2025 16:46:59 -0700 Subject: [PATCH 07/26] [NPE-691] Only support pubsub v3.0+ --- README.md | 6 +- chore-core.gemspec | 2 +- chore.gemspec | 2 +- lib/chore/queues/pubsub.rb | 75 +++++++---- lib/chore/queues/pubsub/consumer.rb | 4 +- lib/chore/queues/pubsub/publisher.rb | 2 +- spec/chore/queues/pubsub/consumer_spec.rb | 12 +- spec/chore/queues/pubsub/publisher_spec.rb | 10 +- spec/chore/queues/pubsub_spec.rb | 140 ++++++++++++++++++--- spec/support/queues/pubsub/fake_objects.rb | 20 ++- 10 files changed, 209 insertions(+), 64 deletions(-) diff --git a/README.md b/README.md index 35f0b99..4f2be47 100644 --- a/README.md +++ b/README.md @@ -26,9 +26,9 @@ Chore can be integrated with any Ruby-based project by following these instructi gem 'aws-sdk-sqs' ``` - For **Google Cloud Pub/Sub**: + For **Google Cloud Pub/Sub** (requires Ruby 3.1+ and google-cloud-pubsub 3.0+): ```ruby - gem 'google-cloud-pubsub', '~> 2.23' + gem 'google-cloud-pubsub', '>= 3.0', '< 4.0' ``` For **Filesystem queues**: No additional gems required. @@ -100,7 +100,7 @@ Chore can authenticate with AWS. **Note**: Publisher configuration (project_id, credentials) is set globally via environment variables or the `Chore::Queues::PubSub.configure` block. Individual jobs cannot override these settings. - **Note**: PubSub requires gRPC which can have threading issues in some environments. If using PubSub in a threaded environment (such as with threaded consumer strategies), ensure you use gRPC version 1.74.1 or higher to avoid potential deadlocks and connection issues. + **Note**: PubSub requires gRPC which can have threading issues in some environments. google-cloud-pubsub 3.0+ includes improved gRPC handling, but if using PubSub in a threaded environment (such as with threaded consumer strategies), ensure you use gRPC version 1.74.1 or higher to avoid potential deadlocks and connection issues. **Warning**: Using different queue types for consumer and publisher (e.g., PubSub consumer with SQS publisher) can be confusing and is generally not recommended. Mixed configurations should only be used carefully for specific scenarios like migrations, validations, or no-op queues. diff --git a/chore-core.gemspec b/chore-core.gemspec index 98f1d9e..1d2a173 100644 --- a/chore-core.gemspec +++ b/chore-core.gemspec @@ -39,7 +39,7 @@ Gem::Specification.new do |s| s.add_runtime_dependency('get_process_mem', [">= 0.2.0"]) s.add_development_dependency(%q, [">= 3"]) s.add_development_dependency(%q, [">= 1"]) - s.add_development_dependency(%q, ["~> 2.23"]) + s.add_development_dependency(%q, [">= 3.0", "< 4.0"]) s.add_development_dependency(%q, [">= 2"]) s.add_development_dependency(%q, [">= 0"]) end diff --git a/chore.gemspec b/chore.gemspec index 98215d5..2844452 100644 --- a/chore.gemspec +++ b/chore.gemspec @@ -40,7 +40,7 @@ Gem::Specification.new do |s| s.add_runtime_dependency(%q, [">= 0"]) s.add_development_dependency(%q, [">= 3"]) s.add_development_dependency(%q, [">= 1"]) - s.add_development_dependency(%q, ["~> 2.23"]) + s.add_development_dependency(%q, [">= 3.0", "< 4.0"]) s.add_development_dependency(%q, [">= 2"]) s.add_development_dependency(%q, [">= 0"]) end diff --git a/lib/chore/queues/pubsub.rb b/lib/chore/queues/pubsub.rb index cc13eb6..112cc69 100644 --- a/lib/chore/queues/pubsub.rb +++ b/lib/chore/queues/pubsub.rb @@ -18,8 +18,8 @@ def self.pubsub_client # Verify compatible version begin gem_version = Gem.loaded_specs['google-cloud-pubsub']&.version - if gem_version && gem_version < Gem::Version.new('2.23.0') - raise "google-cloud-pubsub version #{gem_version} is not supported. Please use version ~> 2.23" + if gem_version && gem_version < Gem::Version.new('3.0.0') + raise "google-cloud-pubsub version #{gem_version} is not supported. Please use version >= 3.0" end rescue => e Chore.logger.warn "Could not verify google-cloud-pubsub version: #{e.message}" if defined?(Chore.logger) @@ -56,16 +56,33 @@ def self.create_queues!(halt_on_existing=false) ERROR end end + client = pubsub_client Chore.prefixed_queue_names.each do |queue_name| Chore.logger.info "Chore Creating Pub/Sub Topic and Subscription: #{queue_name}" - topic = pubsub_client.create_topic(queue_name) + topic_path = client.topic_path(queue_name) subscription_name = "#{queue_name}-sub" + subscription_path = client.subscription_path(subscription_name) + # We rescue in separate blocks because in cases where topic was created + # but the subscription was not, we still want to remove the subscription. + # + # Create topic first. (Reverse on delete) begin - topic.create_subscription(subscription_name) - rescue Google::Cloud::AlreadyExistsError - Chore.logger.info "Subscription #{subscription_name} already exists" + # Create topic using topic admin + client.topic_admin.create_topic(name: topic_path) + rescue Google::Cloud::AlreadyExistsError => e + Chore.logger.info "Topic already exists: #{e}" + end + + begin + # Create subscription using subscription admin + client.subscription_admin.create_subscription( + name: subscription_path, + topic: topic_path + ) + rescue Google::Cloud::AlreadyExistsError => e + Chore.logger.info "Subscription already exists: #{e}" end end @@ -79,21 +96,28 @@ def self.create_queues!(halt_on_existing=false) def self.delete_queues! raise RuntimeError.new('You must have at least one Chore Job before attempting to delete Pub/Sub topics') unless Chore.prefixed_queue_names.length > 0 + client = pubsub_client Chore.prefixed_queue_names.each do |queue_name| + Chore.logger.info "Chore Deleting Pub/Sub Topic and Subscription: #{queue_name}" + subscription_name = "#{queue_name}-sub" + + # We rescue in separate blocks because in cases where subscription was removed + # but the topic was not, we still want to remove the topic. + # + # Delete subscription first + begin + path = client.subscription_path(subscription_name) + client.subscription_admin.delete_subscription(subscription: path) + rescue Google::Cloud::NotFoundError => e + Chore.logger.error "Deleting Subscription: #{queue_name} failed because #{e}" + end + + # Then delete topic begin - Chore.logger.info "Chore Deleting Pub/Sub Topic and Subscription: #{queue_name}" - subscription_name = "#{queue_name}-sub" - - # Delete subscription first - subscription = pubsub_client.subscription(subscription_name) - subscription.delete if subscription.exists? - - # Then delete topic - topic = pubsub_client.topic(queue_name) - topic.delete if topic.exists? - rescue => e - # This could fail for a few reasons - log out why it failed, then continue on - Chore.logger.error "Deleting Topic/Subscription: #{queue_name} failed because #{e}" + path = client.topic_path(queue_name) + client.topic_admin.delete_topic(topic: path) + rescue Google::Cloud::NotFoundError => e + Chore.logger.error "Deleting Topic: #{queue_name} failed because #{e}" end end @@ -106,14 +130,17 @@ def self.delete_queues! def self.existing_queues Chore.prefixed_queue_names.select do |queue_name| begin - topic = pubsub_client.topic(queue_name) - subscription = pubsub_client.subscription("#{queue_name}-sub") - topic.exists? || subscription.exists? - rescue => e + client = pubsub_client + client.publisher(queue_name) + client.subscriber("#{queue_name}-sub") + # if both publisher/subscriber successfully load, then assume exists + true + rescue + # google api throws Google::Cloud::NotFoundError if topic/subscription does not exist false end end end end end -end \ No newline at end of file +end diff --git a/lib/chore/queues/pubsub/consumer.rb b/lib/chore/queues/pubsub/consumer.rb index 2b4445c..1bd8d4a 100644 --- a/lib/chore/queues/pubsub/consumer.rb +++ b/lib/chore/queues/pubsub/consumer.rb @@ -136,14 +136,14 @@ def subscription @subscription = nil end - @subscription ||= pubsub.subscription(@subscription_name) + @subscription ||= pubsub.subscriber(@subscription_name) end # The ack deadline (in seconds) of the subscription # # @return [Integer] def queue_timeout - @queue_timeout ||= subscription.ack_deadline_seconds || 600 # Default to 10 minutes + @queue_timeout ||= subscription.deadline || 600 # Default to 10 minutes end # GCP Pub/Sub client object diff --git a/lib/chore/queues/pubsub/publisher.rb b/lib/chore/queues/pubsub/publisher.rb index dbd9856..7bf34c5 100644 --- a/lib/chore/queues/pubsub/publisher.rb +++ b/lib/chore/queues/pubsub/publisher.rb @@ -54,7 +54,7 @@ def get_topic(name) @pubsub_topics = {} end - @pubsub_topics[name] ||= pubsub.topic(name) + @pubsub_topics[name] ||= pubsub.publisher(name) end end end diff --git a/spec/chore/queues/pubsub/consumer_spec.rb b/spec/chore/queues/pubsub/consumer_spec.rb index 4e1dbfb..277d3d1 100644 --- a/spec/chore/queues/pubsub/consumer_spec.rb +++ b/spec/chore/queues/pubsub/consumer_spec.rb @@ -38,7 +38,7 @@ def consume(&block) end it 'should look up the subscription based on the queue name' do - expect(pubsub_client).to receive(:subscription).with(subscription_name) + expect(pubsub_client).to receive(:subscriber).with(subscription_name) consume end @@ -98,7 +98,7 @@ def consume(&block) received_message.message_id, received_message.ack_id, queue_name, - subscription.ack_deadline_seconds, + subscription.deadline, received_message.data, 0, # delivery_attempt - 1 (1 - 1 = 0) received_timestamp @@ -131,7 +131,7 @@ def consume(&block) received_message.message_id, received_message.ack_id, queue_name, - subscription.ack_deadline_seconds, + subscription.deadline, received_message.data, 2, # delivery_attempt - 1 (3 - 1 = 2) received_timestamp @@ -150,7 +150,7 @@ def consume(&block) received_message.message_id, received_message.ack_id, queue_name, - subscription.ack_deadline_seconds, + subscription.deadline, received_message.data, 0, # (nil || 1) - 1 = 0 received_timestamp @@ -290,8 +290,8 @@ def consume(&block) expect(consumer.send(:queue_timeout)).to eq(600) end - it 'should default to 600 seconds if ack_deadline_seconds is nil' do - allow(subscription).to receive(:ack_deadline_seconds).and_return(nil) + it 'should default to 600 seconds if deadline is nil' do + allow(subscription).to receive(:deadline).and_return(nil) expect(consumer.send(:queue_timeout)).to eq(600) end end diff --git a/spec/chore/queues/pubsub/publisher_spec.rb b/spec/chore/queues/pubsub/publisher_spec.rb index b7b3c93..9364ea5 100644 --- a/spec/chore/queues/pubsub/publisher_spec.rb +++ b/spec/chore/queues/pubsub/publisher_spec.rb @@ -23,7 +23,7 @@ end it 'should lookup the topic when publishing' do - expect(pubsub_client).to receive(:topic).with(queue_name).and_return(topic) + expect(pubsub_client).to receive(:publisher).with(queue_name).and_return(topic) publisher.publish(queue_name, job) end @@ -36,8 +36,8 @@ second_queue_name = queue_name + '2' second_topic = double('Google::Cloud::PubSub::Topic') - expect(pubsub_client).to receive(:topic).with(queue_name).and_return(topic) - expect(pubsub_client).to receive(:topic).with(second_queue_name).and_return(second_topic) + expect(pubsub_client).to receive(:publisher).with(queue_name).and_return(topic) + expect(pubsub_client).to receive(:publisher).with(second_queue_name).and_return(second_topic) expect(topic).to receive(:publish) expect(second_topic).to receive(:publish) @@ -46,7 +46,7 @@ end it 'should only lookup a named topic once' do - expect(pubsub_client).to receive(:topic).with(queue_name).once.and_return(topic) + expect(pubsub_client).to receive(:publisher).with(queue_name).once.and_return(topic) expect(topic).to receive(:publish).exactly(4).times 4.times { publisher.publish(queue_name, job) } end @@ -71,7 +71,7 @@ Chore::Queues::PubSub::Publisher.reset_connection! # Should lookup topic again - expect(pubsub_client).to receive(:topic).with(queue_name).and_return(topic) + expect(pubsub_client).to receive(:publisher).with(queue_name).and_return(topic) publisher.send(:get_topic, queue_name) end diff --git a/spec/chore/queues/pubsub_spec.rb b/spec/chore/queues/pubsub_spec.rb index da0812e..c761b06 100644 --- a/spec/chore/queues/pubsub_spec.rb +++ b/spec/chore/queues/pubsub_spec.rb @@ -10,21 +10,73 @@ context "when managing queues" do it 'should create topics and subscriptions that are defined in its internal job name list' do - expect(pubsub_client).to receive(:create_topic).with(queue_name) - expect(topic).to receive(:create_subscription).with(subscription_name) + topic_admin = double('topic_admin') + subscription_admin = double('subscription_admin') + allow(pubsub_client).to receive(:topic_admin).and_return(topic_admin) + allow(pubsub_client).to receive(:subscription_admin).and_return(subscription_admin) + allow(pubsub_client).to receive(:topic_path).with(queue_name).and_return("projects/test/topics/#{queue_name}") + allow(pubsub_client).to receive(:subscription_path).with(subscription_name).and_return("projects/test/subscriptions/#{subscription_name}") + expect(topic_admin).to receive(:create_topic).with(name: "projects/test/topics/#{queue_name}").and_return(topic) + expect(subscription_admin).to receive(:create_subscription).with( + name: "projects/test/subscriptions/#{subscription_name}", + topic: "projects/test/topics/#{queue_name}" + ) Chore::Queues::PubSub.create_queues! end it 'should delete topics and subscriptions that are defined in its internal job name list' do - expect(pubsub_client).to receive(:subscription).with(subscription_name).and_return(subscription) - expect(pubsub_client).to receive(:topic).with(queue_name).and_return(topic) - expect(subscription).to receive(:delete) - expect(topic).to receive(:delete) + topic_admin = double('topic_admin') + subscription_admin = double('subscription_admin') + allow(pubsub_client).to receive(:topic_admin).and_return(topic_admin) + allow(pubsub_client).to receive(:subscription_admin).and_return(subscription_admin) + allow(pubsub_client).to receive(:topic_path).with(queue_name).and_return("projects/test/topics/#{queue_name}") + allow(pubsub_client).to receive(:subscription_path).with(subscription_name).and_return("projects/test/subscriptions/#{subscription_name}") + + expect(Chore.logger).to receive(:info).with("Chore Deleting Pub/Sub Topic and Subscription: #{queue_name}") + expect(subscription_admin).to receive(:delete_subscription).with(subscription: "projects/test/subscriptions/#{subscription_name}") + expect(topic_admin).to receive(:delete_topic).with(topic: "projects/test/topics/#{queue_name}") + Chore::Queues::PubSub.delete_queues! end + context 'when handling delete errors' do + it 'should continue deleting topic even if subscription delete fails' do + topic_admin = double('topic_admin') + subscription_admin = double('subscription_admin') + allow(pubsub_client).to receive(:topic_admin).and_return(topic_admin) + allow(pubsub_client).to receive(:subscription_admin).and_return(subscription_admin) + allow(pubsub_client).to receive(:topic_path).with(queue_name).and_return("projects/test/topics/#{queue_name}") + allow(pubsub_client).to receive(:subscription_path).with(subscription_name).and_return("projects/test/subscriptions/#{subscription_name}") + + allow(subscription_admin).to receive(:delete_subscription).and_raise(Google::Cloud::NotFoundError.new('Subscription not found')) + expect(topic_admin).to receive(:delete_topic).with(topic: "projects/test/topics/#{queue_name}") + expect(Chore.logger).to receive(:info).with("Chore Deleting Pub/Sub Topic and Subscription: #{queue_name}") + expect(Chore.logger).to receive(:error).with("Deleting Subscription: #{queue_name} failed because Subscription not found") + + Chore::Queues::PubSub.delete_queues! + end + + it 'should continue even if topic delete fails' do + topic_admin = double('topic_admin') + subscription_admin = double('subscription_admin') + allow(pubsub_client).to receive(:topic_admin).and_return(topic_admin) + allow(pubsub_client).to receive(:subscription_admin).and_return(subscription_admin) + allow(pubsub_client).to receive(:topic_path).with(queue_name).and_return("projects/test/topics/#{queue_name}") + allow(pubsub_client).to receive(:subscription_path).with(subscription_name).and_return("projects/test/subscriptions/#{subscription_name}") + + expect(subscription_admin).to receive(:delete_subscription).with(subscription: "projects/test/subscriptions/#{subscription_name}") + allow(topic_admin).to receive(:delete_topic).and_raise(Google::Cloud::NotFoundError.new('Topic not found')) + expect(Chore.logger).to receive(:info).with("Chore Deleting Pub/Sub Topic and Subscription: #{queue_name}") + expect(Chore.logger).to receive(:error).with("Deleting Topic: #{queue_name} failed because Topic not found") + + Chore::Queues::PubSub.delete_queues! + end + end + context 'and checking for existing queues' do it 'checks for existing topics/subscriptions' do + allow(pubsub_client).to receive(:topic_path).with(queue_name).and_return("projects/test/topics/#{queue_name}") + allow(pubsub_client).to receive(:subscription_path).with(subscription_name).and_return("projects/test/subscriptions/#{subscription_name}") expect(described_class).to receive(:existing_queues).and_return([]) Chore::Queues::PubSub.create_queues!(true) end @@ -35,36 +87,90 @@ end it 'does not raise an error if a topic/subscription does not exist' do + topic_admin = double('topic_admin') + subscription_admin = double('subscription_admin') + allow(pubsub_client).to receive(:topic_admin).and_return(topic_admin) + allow(pubsub_client).to receive(:subscription_admin).and_return(subscription_admin) + allow(pubsub_client).to receive(:topic_path).with(queue_name).and_return("projects/test/topics/#{queue_name}") + allow(pubsub_client).to receive(:subscription_path).with(subscription_name).and_return("projects/test/subscriptions/#{subscription_name}") + allow(topic_admin).to receive(:create_topic).and_return(topic) + allow(subscription_admin).to receive(:create_subscription) allow(described_class).to receive(:existing_queues).and_return([]) expect{Chore::Queues::PubSub.create_queues!(true)}.not_to raise_error end end - context 'when handling subscription already exists error' do + context 'when handling already exists errors' do + it 'should continue when topic already exists' do + topic_admin = double('topic_admin') + subscription_admin = double('subscription_admin') + allow(pubsub_client).to receive(:topic_admin).and_return(topic_admin) + allow(pubsub_client).to receive(:subscription_admin).and_return(subscription_admin) + allow(pubsub_client).to receive(:topic_path).with(queue_name).and_return("projects/test/topics/#{queue_name}") + allow(pubsub_client).to receive(:subscription_path).with(subscription_name).and_return("projects/test/subscriptions/#{subscription_name}") + allow(topic_admin).to receive(:create_topic).and_raise(Google::Cloud::AlreadyExistsError.new('Topic already exists')) + allow(subscription_admin).to receive(:create_subscription).and_return(subscription) + expect(Chore.logger).to receive(:info).with("Chore Creating Pub/Sub Topic and Subscription: #{queue_name}") + expect(Chore.logger).to receive(:info).with("Topic already exists: Topic already exists") + expect { Chore::Queues::PubSub.create_queues! }.not_to raise_error + end + it 'should continue when subscription already exists' do - allow(topic).to receive(:create_subscription).and_raise(Google::Cloud::AlreadyExistsError.new('Already exists')) + topic_admin = double('topic_admin') + subscription_admin = double('subscription_admin') + allow(pubsub_client).to receive(:topic_admin).and_return(topic_admin) + allow(pubsub_client).to receive(:subscription_admin).and_return(subscription_admin) + allow(pubsub_client).to receive(:topic_path).with(queue_name).and_return("projects/test/topics/#{queue_name}") + allow(pubsub_client).to receive(:subscription_path).with(subscription_name).and_return("projects/test/subscriptions/#{subscription_name}") + allow(topic_admin).to receive(:create_topic).and_return(topic) + allow(subscription_admin).to receive(:create_subscription).and_raise(Google::Cloud::AlreadyExistsError.new('Subscription already exists')) expect(Chore.logger).to receive(:info).with("Chore Creating Pub/Sub Topic and Subscription: #{queue_name}") - expect(Chore.logger).to receive(:info).with("Subscription #{subscription_name} already exists") + expect(Chore.logger).to receive(:info).with("Subscription already exists: Subscription already exists") + expect { Chore::Queues::PubSub.create_queues! }.not_to raise_error + end + + it 'should continue when both topic and subscription already exist' do + topic_admin = double('topic_admin') + subscription_admin = double('subscription_admin') + allow(pubsub_client).to receive(:topic_admin).and_return(topic_admin) + allow(pubsub_client).to receive(:subscription_admin).and_return(subscription_admin) + allow(pubsub_client).to receive(:topic_path).with(queue_name).and_return("projects/test/topics/#{queue_name}") + allow(pubsub_client).to receive(:subscription_path).with(subscription_name).and_return("projects/test/subscriptions/#{subscription_name}") + allow(topic_admin).to receive(:create_topic).and_raise(Google::Cloud::AlreadyExistsError.new('Topic already exists')) + allow(subscription_admin).to receive(:create_subscription).and_raise(Google::Cloud::AlreadyExistsError.new('Subscription already exists')) + expect(Chore.logger).to receive(:info).with("Chore Creating Pub/Sub Topic and Subscription: #{queue_name}") + expect(Chore.logger).to receive(:info).with("Topic already exists: Topic already exists") + expect(Chore.logger).to receive(:info).with("Subscription already exists: Subscription already exists") expect { Chore::Queues::PubSub.create_queues! }.not_to raise_error end end end describe '.existing_queues' do - it 'returns queues that exist' do - allow(topic).to receive(:exists?).and_return(true) - allow(subscription).to receive(:exists?).and_return(true) + it 'returns queues that exist when both publisher and subscriber calls succeed' do + allow(pubsub_client).to receive(:publisher).with(queue_name).and_return(topic) + allow(pubsub_client).to receive(:subscriber).with("#{queue_name}-sub").and_return(subscription) expect(described_class.existing_queues).to eq([queue_name]) end - it 'filters out queues that do not exist' do - allow(topic).to receive(:exists?).and_return(false) - allow(subscription).to receive(:exists?).and_return(false) + it 'filters out queues when publisher call raises NotFoundError' do + allow(pubsub_client).to receive(:publisher).with(queue_name).and_raise(Google::Cloud::NotFoundError.new('Topic not found')) + expect(described_class.existing_queues).to eq([]) + end + + it 'filters out queues when subscriber call raises NotFoundError' do + allow(pubsub_client).to receive(:publisher).with(queue_name).and_return(topic) + allow(pubsub_client).to receive(:subscriber).with("#{queue_name}-sub").and_raise(Google::Cloud::NotFoundError.new('Subscription not found')) + expect(described_class.existing_queues).to eq([]) + end + + it 'filters out queues when publisher call raises Google::Cloud::NotFoundError' do + allow(pubsub_client).to receive(:publisher).with(queue_name).and_raise(Google::Cloud::NotFoundError.new('Topic not found')) expect(described_class.existing_queues).to eq([]) end - it 'handles errors gracefully' do - allow(pubsub_client).to receive(:topic).and_raise(StandardError.new('Connection error')) + it 'handles other errors gracefully' do + allow(pubsub_client).to receive(:publisher).with(queue_name).and_raise(StandardError.new('Connection error')) expect(described_class.existing_queues).to eq([]) end end diff --git a/spec/support/queues/pubsub/fake_objects.rb b/spec/support/queues/pubsub/fake_objects.rb index df0a2b4..20c15fd 100644 --- a/spec/support/queues/pubsub/fake_objects.rb +++ b/spec/support/queues/pubsub/fake_objects.rb @@ -33,18 +33,30 @@ double('Google::Cloud::PubSub::Subscription', name: subscription_name, exists?: true, - ack_deadline_seconds: 600, + deadline: 600, pull: [received_message], delete: true ) end let(:pubsub_client) do + topic_admin = double('topic_admin', + create_topic: topic, + delete_topic: true + ) + subscription_admin = double('subscription_admin', + create_subscription: subscription, + delete_subscription: true + ) + double('Google::Cloud::PubSub::Project', project_id: project_id, - topic: topic, - subscription: subscription, - create_topic: topic + publisher: topic, + subscriber: subscription, + topic_admin: topic_admin, + subscription_admin: subscription_admin, + topic_path: "projects/#{project_id}/topics/#{queue_name}", + subscription_path: "projects/#{project_id}/subscriptions/#{queue_name}-sub" ) end end From 3225dbe201ea424de35ee13995d41746d79d7d10 Mon Sep 17 00:00:00 2001 From: James Hong Date: Wed, 20 Aug 2025 10:09:39 -0700 Subject: [PATCH 08/26] [NPE-691] Required version changes --- chore.gemspec | 2 +- lib/chore/queues/pubsub.rb | 14 +++++++++----- lib/chore/queues/sqs.rb | 16 ++++++++++++++++ 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/chore.gemspec b/chore.gemspec index 2844452..a7f1422 100644 --- a/chore.gemspec +++ b/chore.gemspec @@ -40,7 +40,7 @@ Gem::Specification.new do |s| s.add_runtime_dependency(%q, [">= 0"]) s.add_development_dependency(%q, [">= 3"]) s.add_development_dependency(%q, [">= 1"]) - s.add_development_dependency(%q, [">= 3.0", "< 4.0"]) + s.add_development_dependency(%q, [">= 3.0"]) s.add_development_dependency(%q, [">= 2"]) s.add_development_dependency(%q, [">= 0"]) end diff --git a/lib/chore/queues/pubsub.rb b/lib/chore/queues/pubsub.rb index 112cc69..8b76f79 100644 --- a/lib/chore/queues/pubsub.rb +++ b/lib/chore/queues/pubsub.rb @@ -10,19 +10,23 @@ def configure end end + REQUIRED_LIBRARY = "google/cloud/pubsub".freeze + MIN_VERSION = Gem::Version.new('3.0.0') + # Creates a configured PubSub client with the given options # Creates a GCP Pub/Sub client using global configuration def self.pubsub_client - require 'google/cloud/pubsub' + require REQUIRED_LIBRARY # Verify compatible version begin - gem_version = Gem.loaded_specs['google-cloud-pubsub']&.version - if gem_version && gem_version < Gem::Version.new('3.0.0') - raise "google-cloud-pubsub version #{gem_version} is not supported. Please use version >= 3.0" + gem_version = Gem::Version.new(Google::Cloud::PubSub::VERSION) + if gem_version && gem_version < MIN_VERSION + raise "#{REQUIRED_LIBRARY} version #{gem_version} is not supported. Please use version >= #{MIN_VERSION}" end rescue => e - Chore.logger.warn "Could not verify google-cloud-pubsub version: #{e.message}" if defined?(Chore.logger) + Chore.logger.error "Could not verify #{REQUIRED_LIBRARY} version: #{e.message}" if defined?(Chore.logger) + exit end if self.project_id && self.credentials diff --git a/lib/chore/queues/sqs.rb b/lib/chore/queues/sqs.rb index 2ec55e8..1694fb9 100644 --- a/lib/chore/queues/sqs.rb +++ b/lib/chore/queues/sqs.rb @@ -1,7 +1,23 @@ module Chore module Queues module SQS + REQUIRED_LIBRARY = "aws-sdk-sqs".freeze + MIN_VERSION = Gem::Version.new("1") + def self.sqs_client + require REQUIRED_LIBRARY + + # Verify compatible version + begin + gem_version = Gem::Version.new(Aws::SQS::GEM_VERSION) + if gem_version < MIN_VERSION + raise "#{REQUIRED_LIBRARY} version #{gem_version} is not supported. Please use version >= #{MIN_VERSION}" + end + rescue => e + Chore.logger.error "Could not verify #{REQUIRED_LIBRARY} version: #{e.message}" if defined?(Chore.logger) + exit + end + Aws::SQS::Client.new( logger: Chore.logger, log_level: Chore.log_level_to_sym, From 1e31c48012ce483085ed6ba86cfb415f41957e26 Mon Sep 17 00:00:00 2001 From: James Hong Date: Wed, 20 Aug 2025 10:09:50 -0700 Subject: [PATCH 09/26] [NPE-691] Remove legacy tasks --- lib/chore/tasks/queues.task | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/lib/chore/tasks/queues.task b/lib/chore/tasks/queues.task index c4d9623..4743529 100644 --- a/lib/chore/tasks/queues.task +++ b/lib/chore/tasks/queues.task @@ -32,15 +32,4 @@ namespace :chore do Chore.logger.info "Successfully deleted #{deleted_queues.length} Pub/Sub topics: #{deleted_queues.join(', ')}" end end - - # Legacy tasks for backward compatibility - desc "Create all defined SQS queues (legacy - use chore:queues:create_sqs instead)" - task :create, :halt_on_existing do |t, args| - Rake::Task['chore:queues:create_sqs'].invoke(args[:halt_on_existing]) - end - - desc "Remove all defined SQS queues (legacy - use chore:queues:remove_sqs instead)" - task :remove do - Rake::Task['chore:queues:remove_sqs'].invoke - end end From 0b0097ab00573edacb0b4d0472f823a50a883bb3 Mon Sep 17 00:00:00 2001 From: James Hong Date: Wed, 20 Aug 2025 10:10:28 -0700 Subject: [PATCH 10/26] [NPE-691] Remove Chore::Queues::PubSub configurations --- lib/chore/queues/pubsub.rb | 20 ++------------------ lib/chore/queues/pubsub/consumer.rb | 8 -------- 2 files changed, 2 insertions(+), 26 deletions(-) diff --git a/lib/chore/queues/pubsub.rb b/lib/chore/queues/pubsub.rb index 8b76f79..8832a9d 100644 --- a/lib/chore/queues/pubsub.rb +++ b/lib/chore/queues/pubsub.rb @@ -1,15 +1,6 @@ module Chore module Queues module PubSub - class << self - attr_accessor :project_id, :credentials - - # Configure PubSub settings with a block - def configure - yield self if block_given? - end - end - REQUIRED_LIBRARY = "google/cloud/pubsub".freeze MIN_VERSION = Gem::Version.new('3.0.0') @@ -29,14 +20,7 @@ def self.pubsub_client exit end - if self.project_id && self.credentials - Google::Cloud::PubSub.new( - project_id: self.project_id, - credentials: self.credentials - ) - else - Google::Cloud::PubSub.new - end + Google::Cloud::PubSub.new end # Helper method to create topics and subscriptions based on the currently known list as provided by your configured Chore::Jobs # This is meant to be invoked from a rake task, and not directly. @@ -132,9 +116,9 @@ def self.delete_queues! # # @return [Array] def self.existing_queues + client = pubsub_client Chore.prefixed_queue_names.select do |queue_name| begin - client = pubsub_client client.publisher(queue_name) client.subscriber("#{queue_name}-sub") # if both publisher/subscriber successfully load, then assume exists diff --git a/lib/chore/queues/pubsub/consumer.rb b/lib/chore/queues/pubsub/consumer.rb index 1bd8d4a..83cb118 100644 --- a/lib/chore/queues/pubsub/consumer.rb +++ b/lib/chore/queues/pubsub/consumer.rb @@ -10,14 +10,6 @@ class Consumer < Chore::Consumer # Initialize the reset at on class load @@reset_at = Time.now - Chore::CLI.register_option 'gcp_project_id', '--gcp-project-id PROJECT_ID', 'GCP Project ID for Pub/Sub' do |project_id| - Chore::Queues::PubSub.project_id = project_id - end - - Chore::CLI.register_option 'gcp_credentials', '--gcp-credentials PATH', 'Path to GCP service account credentials JSON file' do |credentials| - Chore::Queues::PubSub.credentials = credentials - end - # Resets the API client connection and provides @@reset_at so we know when the last time that was done def self.reset_connection! @@reset_at = Time.now From 6123a9c70f28db56d6c41c68e57d7795f478d97e Mon Sep 17 00:00:00 2001 From: James Hong Date: Wed, 20 Aug 2025 10:11:00 -0700 Subject: [PATCH 11/26] [NPE-691] Use Chore's polling amount for PubSub --- lib/chore/queues/pubsub/consumer.rb | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/lib/chore/queues/pubsub/consumer.rb b/lib/chore/queues/pubsub/consumer.rb index 83cb118..ffcd2b0 100644 --- a/lib/chore/queues/pubsub/consumer.rb +++ b/lib/chore/queues/pubsub/consumer.rb @@ -99,7 +99,7 @@ def handle_messages(&block) raise Chore::TerribleMistake end - messages = subscription.pull(max: max_messages) + messages = subscription.pull(max: pubsub_polling_amount) @current_messages = messages received_timestamp = Time.now @@ -145,13 +145,6 @@ def pubsub @pubsub ||= Chore::Queues::PubSub.pubsub_client end - # Maximum number of messages to retrieve on each request - # - # @return [Integer] - def max_messages - [pubsub_polling_amount, 1000].min # Pub/Sub max is 1000 - end - # Maximum number of messages to retrieve on each request from config # # @return [Integer] From 7f577c637cf2aa5923a3b2bb5ac4507fdb996c85 Mon Sep 17 00:00:00 2001 From: James Hong Date: Wed, 20 Aug 2025 10:11:50 -0700 Subject: [PATCH 12/26] [NPE-691] Cleanup code --- README.md | 6 +++--- lib/chore/queues/pubsub/consumer.rb | 23 ++++++++++------------ spec/chore/queues/pubsub/consumer_spec.rb | 18 +++++------------ spec/support/queues/pubsub/fake_objects.rb | 4 +--- 4 files changed, 19 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index 4f2be47..4c009f9 100644 --- a/README.md +++ b/README.md @@ -355,9 +355,9 @@ Read more details about SQS and Delays [here](docs/Delayed%20Jobs.md) GCP Pub/Sub uses a topic and subscription model. When using Chore with Pub/Sub: * Each queue corresponds to a Pub/Sub topic -* Subscriptions are automatically created with the naming pattern `{topic-name}-sub` -* Message delays are handled using `modify_ack_deadline!` instead of SQS visibility timeouts -* Messages are acknowledged using `acknowledge!` instead of being deleted +* Subscriptions are automatically created with the naming pattern `{topic-name}` +* Message delays are handled using subscription-level ack deadline modification instead of SQS visibility timeouts +* Messages are acknowledged using subscription-level acknowledgment instead of being deleted * Pull-based consumption is used with configurable batch sizes (up to 1000 messages) ## Hooks diff --git a/lib/chore/queues/pubsub/consumer.rb b/lib/chore/queues/pubsub/consumer.rb index ffcd2b0..ff46ad7 100644 --- a/lib/chore/queues/pubsub/consumer.rb +++ b/lib/chore/queues/pubsub/consumer.rb @@ -59,11 +59,9 @@ def reject(message_id) # @param [String] message_id Unique ID of the Pub/Sub message # @param [String] ack_id Acknowledgment ID of the Pub/Sub message def complete(message_id, ack_id) - Chore.logger.debug "Completing (acknowledging): #{message_id}" + Chore.logger.debug "Completing (acknowledging): #{message_id} ack_id: #{ack_id}" # Find the message by ack_id and acknowledge it - if msg = @current_messages&.find { |m| m.ack_id == ack_id } - msg.acknowledge! - end + @subscription.acknowledge(ack_id) end # Delays retry of a job by +backoff_calc+ seconds. @@ -76,9 +74,7 @@ def delay(item, backoff_calc) Chore.logger.debug "Delaying #{item.id} by #{delay} seconds" # Find the message and modify its ack deadline - if msg = @current_messages&.find { |m| m.ack_id == item.receipt_handle } - msg.modify_ack_deadline!(delay) - end + @subscription.modify_ack_deadline(delay, item.receipt_handle) return delay end @@ -100,16 +96,17 @@ def handle_messages(&block) end messages = subscription.pull(max: pubsub_polling_amount) - @current_messages = messages received_timestamp = Time.now messages.each do |message| - unless duplicate_message?(message.message_id, @subscription_name, queue_timeout, received_timestamp) - # delivery_attempt is available but may be nil for older messages - attempt_count = (message.delivery_attempt || 1) - 1 - block.call(message.message_id, message.ack_id, queue_name, queue_timeout, message.data, attempt_count, received_timestamp) + begin + unless duplicate_message?(message.message_id, @subscription_name, queue_timeout, received_timestamp) + # delivery_attempt is available but may be nil for older messages + attempt_count = (message.delivery_attempt || 1) - 1 + block.call(message.message_id, message.ack_id, queue_name, queue_timeout, message.data, attempt_count, received_timestamp) + end + Chore.run_hooks_for(:on_fetch, message.ack_id, message.data) end - Chore.run_hooks_for(:on_fetch, message.ack_id, message.data) end messages diff --git a/spec/chore/queues/pubsub/consumer_spec.rb b/spec/chore/queues/pubsub/consumer_spec.rb index 277d3d1..b40e798 100644 --- a/spec/chore/queues/pubsub/consumer_spec.rb +++ b/spec/chore/queues/pubsub/consumer_spec.rb @@ -188,16 +188,13 @@ def consume(&block) expect { consume }.to raise_error(Chore::TerribleMistake) end end + + end describe "completing work" do - before do - # Set up current messages for the complete method to work - consumer.instance_variable_set(:@current_messages, [received_message]) - end - it 'acknowledges the message in the subscription' do - expect(received_message).to receive(:acknowledge!) + expect(subscription).to receive(:acknowledge).with(received_message.ack_id) consumer.complete(received_message.message_id, received_message.ack_id) end @@ -209,14 +206,9 @@ def consume(&block) describe '#delay' do let(:item) { Chore::UnitOfWork.new(received_message.message_id, received_message.ack_id, queue_name, 600, received_message.data, 0, consumer) } - before do - # Set up current messages for the delay method to work - consumer.instance_variable_set(:@current_messages, [received_message]) - end - it 'modifies the ack deadline of the message' do delay_seconds = backoff_func.call(item) - expect(received_message).to receive(:modify_ack_deadline!).with(delay_seconds) + expect(subscription).to receive(:modify_ack_deadline).with(delay_seconds, received_message.ack_id) consumer.delay(item, backoff_func) end @@ -295,4 +287,4 @@ def consume(&block) expect(consumer.send(:queue_timeout)).to eq(600) end end -end \ No newline at end of file +end diff --git a/spec/support/queues/pubsub/fake_objects.rb b/spec/support/queues/pubsub/fake_objects.rb index 20c15fd..79ac196 100644 --- a/spec/support/queues/pubsub/fake_objects.rb +++ b/spec/support/queues/pubsub/fake_objects.rb @@ -13,9 +13,7 @@ message_id: message_id, ack_id: ack_id, data: message_data, - delivery_attempt: 1, - acknowledge!: true, - modify_ack_deadline!: true + delivery_attempt: 1 ) end From 8a2447d6b28e7cf757f74c7af992f3d1e05c0ab2 Mon Sep 17 00:00:00 2001 From: James Hong Date: Fri, 22 Aug 2025 17:28:31 -0700 Subject: [PATCH 13/26] [NPE-691] Fix specs --- lib/chore/queues/pubsub/consumer.rb | 7 ++++--- spec/support/queues/pubsub/fake_objects.rb | 4 +++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/chore/queues/pubsub/consumer.rb b/lib/chore/queues/pubsub/consumer.rb index ff46ad7..7611f83 100644 --- a/lib/chore/queues/pubsub/consumer.rb +++ b/lib/chore/queues/pubsub/consumer.rb @@ -61,7 +61,7 @@ def reject(message_id) def complete(message_id, ack_id) Chore.logger.debug "Completing (acknowledging): #{message_id} ack_id: #{ack_id}" # Find the message by ack_id and acknowledge it - @subscription.acknowledge(ack_id) + subscription.acknowledge(ack_id) end # Delays retry of a job by +backoff_calc+ seconds. @@ -74,7 +74,7 @@ def delay(item, backoff_calc) Chore.logger.debug "Delaying #{item.id} by #{delay} seconds" # Find the message and modify its ack deadline - @subscription.modify_ack_deadline(delay, item.receipt_handle) + subscription.modify_ack_deadline(delay, item.receipt_handle) return delay end @@ -143,10 +143,11 @@ def pubsub end # Maximum number of messages to retrieve on each request from config + # Capped at 1000 messages which is the Pub/Sub limit # # @return [Integer] def pubsub_polling_amount - Chore.config.queue_polling_size + [Chore.config.queue_polling_size, 1000].min end end end diff --git a/spec/support/queues/pubsub/fake_objects.rb b/spec/support/queues/pubsub/fake_objects.rb index 79ac196..2753fab 100644 --- a/spec/support/queues/pubsub/fake_objects.rb +++ b/spec/support/queues/pubsub/fake_objects.rb @@ -33,7 +33,9 @@ exists?: true, deadline: 600, pull: [received_message], - delete: true + delete: true, + acknowledge: true, + modify_ack_deadline: true ) end From 3e150204cc903db21e544323e59ca9de4ec58346 Mon Sep 17 00:00:00 2001 From: James Hong Date: Fri, 22 Aug 2025 17:35:26 -0700 Subject: [PATCH 14/26] [NPE-691] Update Gemfile.lock files --- Gemfile.latest.lock | 88 +++++++++++++++++++++++++++++++++++++++++++-- Gemfile.ruby31.lock | 88 +++++++++++++++++++++++++++++++++++++++++++-- Gemfile.ruby32.lock | 88 +++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 258 insertions(+), 6 deletions(-) diff --git a/Gemfile.latest.lock b/Gemfile.latest.lock index d67038e..a4a3155 100644 --- a/Gemfile.latest.lock +++ b/Gemfile.latest.lock @@ -1,14 +1,15 @@ PATH remote: . specs: - chore-core (4.7.0) - aws-sdk-sqs (>= 1) + chore-core (5.0.0) get_process_mem (>= 0.2.0) multi_json GEM remote: https://rubygems.org/ specs: + addressable (2.8.7) + public_suffix (>= 2.0.2, < 7.0) aws-eventstream (1.3.0) aws-partitions (1.983.0) aws-sdk-core (3.209.1) @@ -21,15 +22,90 @@ GEM aws-sigv4 (~> 1.5) aws-sigv4 (1.10.0) aws-eventstream (~> 1, >= 1.0.2) + base64 (0.3.0) bigdecimal (3.1.8) + concurrent-ruby (1.3.5) dalli (3.2.8) diff-lcs (1.5.1) + faraday (2.13.4) + faraday-net_http (>= 2.0, < 3.5) + json + logger + faraday-net_http (3.4.1) + net-http (>= 0.5.0) + faraday-retry (2.3.2) + faraday (~> 2.0) ffi (1.17.0) + gapic-common (1.1.0) + faraday (>= 1.9, < 3.a) + faraday-retry (>= 1.0, < 3.a) + google-cloud-env (~> 2.2) + google-logging-utils (~> 0.1) + google-protobuf (>= 3.25, < 5.a) + googleapis-common-protos (~> 1.6) + googleapis-common-protos-types (~> 1.15) + googleauth (~> 1.12) + grpc (~> 1.66) get_process_mem (1.0.0) bigdecimal (>= 2.0) ffi (~> 1.0) + google-cloud-core (1.8.0) + google-cloud-env (>= 1.0, < 3.a) + google-cloud-errors (~> 1.0) + google-cloud-env (2.3.1) + base64 (~> 0.2) + faraday (>= 1.0, < 3.a) + google-cloud-errors (1.5.0) + google-cloud-pubsub (3.0.2) + concurrent-ruby (~> 1.3) + google-cloud-core (~> 1.8) + google-cloud-pubsub-v1 (~> 1.11) + retriable (~> 3.1) + google-cloud-pubsub-v1 (1.11.1) + gapic-common (~> 1.0) + google-cloud-errors (~> 1.0) + google-iam-v1 (~> 1.3) + google-iam-v1 (1.4.0) + gapic-common (~> 1.0) + google-cloud-errors (~> 1.0) + grpc-google-iam-v1 (~> 1.11) + google-logging-utils (0.2.0) + google-protobuf (4.32.0) + bigdecimal + rake (>= 13) + googleapis-common-protos (1.7.0) + google-protobuf (>= 3.18, < 5.a) + googleapis-common-protos-types (~> 1.7) + grpc (~> 1.41) + googleapis-common-protos-types (1.20.0) + google-protobuf (>= 3.18, < 5.a) + googleauth (1.14.0) + faraday (>= 1.0, < 3.a) + google-cloud-env (~> 2.2) + google-logging-utils (~> 0.1) + jwt (>= 1.4, < 3.0) + multi_json (~> 1.11) + os (>= 0.9, < 2.0) + signet (>= 0.16, < 2.a) + grpc (1.74.1) + google-protobuf (>= 3.25, < 5.0) + googleapis-common-protos-types (~> 1.0) + grpc-google-iam-v1 (1.11.0) + google-protobuf (>= 3.18, < 5.a) + googleapis-common-protos (~> 1.7.0) + grpc (~> 1.41) jmespath (1.6.2) + json (2.13.2) + jwt (2.10.2) + base64 + logger (1.7.0) multi_json (1.15.0) + net-http (0.6.0) + uri + os (1.1.4) + public_suffix (6.0.2) + rake (13.3.0) + retriable (3.1.2) rspec (3.13.0) rspec-core (~> 3.13.0) rspec-expectations (~> 3.13.0) @@ -43,14 +119,22 @@ GEM diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.13.0) rspec-support (3.13.1) + signet (0.20.0) + addressable (~> 2.8) + faraday (>= 0.17.5, < 3.a) + jwt (>= 1.5, < 3.0) + multi_json (~> 1.10) timecop (0.9.10) + uri (1.0.3) PLATFORMS ruby DEPENDENCIES + aws-sdk-sqs (>= 1) chore-core! dalli (>= 2) + google-cloud-pubsub (>= 3.0, < 4.0) rspec (>= 3) timecop diff --git a/Gemfile.ruby31.lock b/Gemfile.ruby31.lock index 76517a7..8ba829d 100644 --- a/Gemfile.ruby31.lock +++ b/Gemfile.ruby31.lock @@ -1,14 +1,15 @@ PATH remote: . specs: - chore-core (4.7.0) - aws-sdk-sqs (>= 1) + chore-core (5.0.0) get_process_mem (>= 0.2.0) multi_json GEM remote: https://rubygems.org/ specs: + addressable (2.8.7) + public_suffix (>= 2.0.2, < 7.0) aws-eventstream (1.3.0) aws-partitions (1.983.0) aws-sdk-core (3.209.1) @@ -21,15 +22,90 @@ GEM aws-sigv4 (~> 1.5) aws-sigv4 (1.10.0) aws-eventstream (~> 1, >= 1.0.2) + base64 (0.3.0) bigdecimal (3.1.8) + concurrent-ruby (1.3.5) dalli (3.2.8) diff-lcs (1.5.1) + faraday (2.13.4) + faraday-net_http (>= 2.0, < 3.5) + json + logger + faraday-net_http (3.4.1) + net-http (>= 0.5.0) + faraday-retry (2.3.2) + faraday (~> 2.0) ffi (1.17.0) + gapic-common (1.1.0) + faraday (>= 1.9, < 3.a) + faraday-retry (>= 1.0, < 3.a) + google-cloud-env (~> 2.2) + google-logging-utils (~> 0.1) + google-protobuf (>= 3.25, < 5.a) + googleapis-common-protos (~> 1.6) + googleapis-common-protos-types (~> 1.15) + googleauth (~> 1.12) + grpc (~> 1.66) get_process_mem (1.0.0) bigdecimal (>= 2.0) ffi (~> 1.0) + google-cloud-core (1.8.0) + google-cloud-env (>= 1.0, < 3.a) + google-cloud-errors (~> 1.0) + google-cloud-env (2.3.1) + base64 (~> 0.2) + faraday (>= 1.0, < 3.a) + google-cloud-errors (1.5.0) + google-cloud-pubsub (3.0.2) + concurrent-ruby (~> 1.3) + google-cloud-core (~> 1.8) + google-cloud-pubsub-v1 (~> 1.11) + retriable (~> 3.1) + google-cloud-pubsub-v1 (1.11.1) + gapic-common (~> 1.0) + google-cloud-errors (~> 1.0) + google-iam-v1 (~> 1.3) + google-iam-v1 (1.4.0) + gapic-common (~> 1.0) + google-cloud-errors (~> 1.0) + grpc-google-iam-v1 (~> 1.11) + google-logging-utils (0.2.0) + google-protobuf (4.32.0) + bigdecimal + rake (>= 13) + googleapis-common-protos (1.7.0) + google-protobuf (>= 3.18, < 5.a) + googleapis-common-protos-types (~> 1.7) + grpc (~> 1.41) + googleapis-common-protos-types (1.20.0) + google-protobuf (>= 3.18, < 5.a) + googleauth (1.14.0) + faraday (>= 1.0, < 3.a) + google-cloud-env (~> 2.2) + google-logging-utils (~> 0.1) + jwt (>= 1.4, < 3.0) + multi_json (~> 1.11) + os (>= 0.9, < 2.0) + signet (>= 0.16, < 2.a) + grpc (1.74.1) + google-protobuf (>= 3.25, < 5.0) + googleapis-common-protos-types (~> 1.0) + grpc-google-iam-v1 (1.11.0) + google-protobuf (>= 3.18, < 5.a) + googleapis-common-protos (~> 1.7.0) + grpc (~> 1.41) jmespath (1.6.2) + json (2.13.2) + jwt (2.10.2) + base64 + logger (1.7.0) multi_json (1.15.0) + net-http (0.6.0) + uri + os (1.1.4) + public_suffix (6.0.2) + rake (13.3.0) + retriable (3.1.2) rspec (3.13.0) rspec-core (~> 3.13.0) rspec-expectations (~> 3.13.0) @@ -43,14 +119,22 @@ GEM diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.13.0) rspec-support (3.13.1) + signet (0.20.0) + addressable (~> 2.8) + faraday (>= 0.17.5, < 3.a) + jwt (>= 1.5, < 3.0) + multi_json (~> 1.10) timecop (0.9.10) + uri (1.0.3) PLATFORMS ruby DEPENDENCIES + aws-sdk-sqs (>= 1) chore-core! dalli (>= 2) + google-cloud-pubsub (>= 3.0, < 4.0) rspec (>= 3) timecop diff --git a/Gemfile.ruby32.lock b/Gemfile.ruby32.lock index 9216534..e0b925b 100644 --- a/Gemfile.ruby32.lock +++ b/Gemfile.ruby32.lock @@ -1,14 +1,15 @@ PATH remote: . specs: - chore-core (4.7.0) - aws-sdk-sqs (>= 1) + chore-core (5.0.0) get_process_mem (>= 0.2.0) multi_json GEM remote: https://rubygems.org/ specs: + addressable (2.8.7) + public_suffix (>= 2.0.2, < 7.0) aws-eventstream (1.3.0) aws-partitions (1.983.0) aws-sdk-core (3.209.1) @@ -21,15 +22,90 @@ GEM aws-sigv4 (~> 1.5) aws-sigv4 (1.10.0) aws-eventstream (~> 1, >= 1.0.2) + base64 (0.3.0) bigdecimal (3.1.8) + concurrent-ruby (1.3.5) dalli (3.2.8) diff-lcs (1.5.1) + faraday (2.13.4) + faraday-net_http (>= 2.0, < 3.5) + json + logger + faraday-net_http (3.4.1) + net-http (>= 0.5.0) + faraday-retry (2.3.2) + faraday (~> 2.0) ffi (1.17.0) + gapic-common (1.1.0) + faraday (>= 1.9, < 3.a) + faraday-retry (>= 1.0, < 3.a) + google-cloud-env (~> 2.2) + google-logging-utils (~> 0.1) + google-protobuf (>= 3.25, < 5.a) + googleapis-common-protos (~> 1.6) + googleapis-common-protos-types (~> 1.15) + googleauth (~> 1.12) + grpc (~> 1.66) get_process_mem (1.0.0) bigdecimal (>= 2.0) ffi (~> 1.0) + google-cloud-core (1.8.0) + google-cloud-env (>= 1.0, < 3.a) + google-cloud-errors (~> 1.0) + google-cloud-env (2.3.1) + base64 (~> 0.2) + faraday (>= 1.0, < 3.a) + google-cloud-errors (1.5.0) + google-cloud-pubsub (3.0.2) + concurrent-ruby (~> 1.3) + google-cloud-core (~> 1.8) + google-cloud-pubsub-v1 (~> 1.11) + retriable (~> 3.1) + google-cloud-pubsub-v1 (1.11.1) + gapic-common (~> 1.0) + google-cloud-errors (~> 1.0) + google-iam-v1 (~> 1.3) + google-iam-v1 (1.4.0) + gapic-common (~> 1.0) + google-cloud-errors (~> 1.0) + grpc-google-iam-v1 (~> 1.11) + google-logging-utils (0.2.0) + google-protobuf (4.32.0) + bigdecimal + rake (>= 13) + googleapis-common-protos (1.7.0) + google-protobuf (>= 3.18, < 5.a) + googleapis-common-protos-types (~> 1.7) + grpc (~> 1.41) + googleapis-common-protos-types (1.20.0) + google-protobuf (>= 3.18, < 5.a) + googleauth (1.14.0) + faraday (>= 1.0, < 3.a) + google-cloud-env (~> 2.2) + google-logging-utils (~> 0.1) + jwt (>= 1.4, < 3.0) + multi_json (~> 1.11) + os (>= 0.9, < 2.0) + signet (>= 0.16, < 2.a) + grpc (1.74.1) + google-protobuf (>= 3.25, < 5.0) + googleapis-common-protos-types (~> 1.0) + grpc-google-iam-v1 (1.11.0) + google-protobuf (>= 3.18, < 5.a) + googleapis-common-protos (~> 1.7.0) + grpc (~> 1.41) jmespath (1.6.2) + json (2.13.2) + jwt (2.10.2) + base64 + logger (1.7.0) multi_json (1.15.0) + net-http (0.6.0) + uri + os (1.1.4) + public_suffix (6.0.2) + rake (13.3.0) + retriable (3.1.2) rspec (3.13.0) rspec-core (~> 3.13.0) rspec-expectations (~> 3.13.0) @@ -43,14 +119,22 @@ GEM diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.13.0) rspec-support (3.13.1) + signet (0.20.0) + addressable (~> 2.8) + faraday (>= 0.17.5, < 3.a) + jwt (>= 1.5, < 3.0) + multi_json (~> 1.10) timecop (0.9.10) + uri (1.0.3) PLATFORMS ruby DEPENDENCIES + aws-sdk-sqs (>= 1) chore-core! dalli (>= 2) + google-cloud-pubsub (>= 3.0, < 4.0) rspec (>= 3) timecop From 2a120247250e7506431e2e6139148b3d6138eed6 Mon Sep 17 00:00:00 2001 From: James Hong Date: Wed, 27 Aug 2025 14:08:45 -0700 Subject: [PATCH 15/26] [NPE-691] Remove -sub suffix from sub-name --- README.md | 2 +- lib/chore/queues/pubsub.rb | 8 +++----- lib/chore/queues/pubsub/consumer.rb | 2 +- spec/chore/queues/pubsub_spec.rb | 6 +++--- spec/support/queues/pubsub/fake_objects.rb | 6 +++--- 5 files changed, 11 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 4c009f9..e41e118 100644 --- a/README.md +++ b/README.md @@ -215,7 +215,7 @@ This section assumes `foreman` is being used to execute (or export the run comma require 'google/cloud/pubsub' pubsub = Google::Cloud::PubSub.new topic = pubsub.create_topic "test_queue" - topic.create_subscription "test_queue-sub" + topic.create_subscription "test_queue" ``` Alternatively, you can use Chore's built-in queue management: diff --git a/lib/chore/queues/pubsub.rb b/lib/chore/queues/pubsub.rb index 8832a9d..13b24e9 100644 --- a/lib/chore/queues/pubsub.rb +++ b/lib/chore/queues/pubsub.rb @@ -49,8 +49,7 @@ def self.create_queues!(halt_on_existing=false) Chore.prefixed_queue_names.each do |queue_name| Chore.logger.info "Chore Creating Pub/Sub Topic and Subscription: #{queue_name}" topic_path = client.topic_path(queue_name) - subscription_name = "#{queue_name}-sub" - subscription_path = client.subscription_path(subscription_name) + subscription_path = client.subscription_path(queue_name) # We rescue in separate blocks because in cases where topic was created # but the subscription was not, we still want to remove the subscription. @@ -87,14 +86,13 @@ def self.delete_queues! client = pubsub_client Chore.prefixed_queue_names.each do |queue_name| Chore.logger.info "Chore Deleting Pub/Sub Topic and Subscription: #{queue_name}" - subscription_name = "#{queue_name}-sub" # We rescue in separate blocks because in cases where subscription was removed # but the topic was not, we still want to remove the topic. # # Delete subscription first begin - path = client.subscription_path(subscription_name) + path = client.subscription_path(queue_name) client.subscription_admin.delete_subscription(subscription: path) rescue Google::Cloud::NotFoundError => e Chore.logger.error "Deleting Subscription: #{queue_name} failed because #{e}" @@ -120,7 +118,7 @@ def self.existing_queues Chore.prefixed_queue_names.select do |queue_name| begin client.publisher(queue_name) - client.subscriber("#{queue_name}-sub") + client.subscriber(queue_name) # if both publisher/subscriber successfully load, then assume exists true rescue diff --git a/lib/chore/queues/pubsub/consumer.rb b/lib/chore/queues/pubsub/consumer.rb index 7611f83..060d482 100644 --- a/lib/chore/queues/pubsub/consumer.rb +++ b/lib/chore/queues/pubsub/consumer.rb @@ -19,7 +19,7 @@ def self.reset_connection! # @param [Hash] opts Options def initialize(queue_name, opts={}) super(queue_name, opts) - @subscription_name = "#{queue_name}-sub" + @subscription_name = queue_name end # Ensure that the consumer is capable of running diff --git a/spec/chore/queues/pubsub_spec.rb b/spec/chore/queues/pubsub_spec.rb index c761b06..ec319db 100644 --- a/spec/chore/queues/pubsub_spec.rb +++ b/spec/chore/queues/pubsub_spec.rb @@ -149,7 +149,7 @@ describe '.existing_queues' do it 'returns queues that exist when both publisher and subscriber calls succeed' do allow(pubsub_client).to receive(:publisher).with(queue_name).and_return(topic) - allow(pubsub_client).to receive(:subscriber).with("#{queue_name}-sub").and_return(subscription) + allow(pubsub_client).to receive(:subscriber).with(queue_name).and_return(subscription) expect(described_class.existing_queues).to eq([queue_name]) end @@ -160,7 +160,7 @@ it 'filters out queues when subscriber call raises NotFoundError' do allow(pubsub_client).to receive(:publisher).with(queue_name).and_return(topic) - allow(pubsub_client).to receive(:subscriber).with("#{queue_name}-sub").and_raise(Google::Cloud::NotFoundError.new('Subscription not found')) + allow(pubsub_client).to receive(:subscriber).with(queue_name).and_raise(Google::Cloud::NotFoundError.new('Subscription not found')) expect(described_class.existing_queues).to eq([]) end @@ -174,4 +174,4 @@ expect(described_class.existing_queues).to eq([]) end end -end \ No newline at end of file +end diff --git a/spec/support/queues/pubsub/fake_objects.rb b/spec/support/queues/pubsub/fake_objects.rb index 2753fab..9a91649 100644 --- a/spec/support/queues/pubsub/fake_objects.rb +++ b/spec/support/queues/pubsub/fake_objects.rb @@ -1,7 +1,7 @@ describe Chore::Queues::PubSub do RSpec.shared_context 'fake pubsub objects' do let(:queue_name) { 'test_queue' } - let(:subscription_name) { "#{queue_name}-sub" } + let(:subscription_name) { queue_name } let(:project_id) { 'test-project' } let(:message_data) { {'class' => 'TestJob', 'args' => [1, 2, '3']}.to_json } @@ -56,8 +56,8 @@ topic_admin: topic_admin, subscription_admin: subscription_admin, topic_path: "projects/#{project_id}/topics/#{queue_name}", - subscription_path: "projects/#{project_id}/subscriptions/#{queue_name}-sub" + subscription_path: "projects/#{project_id}/subscriptions/#{queue_name}" ) end end -end \ No newline at end of file +end From 26561b734b84be59ed511e92973a334c18866ba5 Mon Sep 17 00:00:00 2001 From: James Hong Date: Wed, 27 Aug 2025 14:09:32 -0700 Subject: [PATCH 16/26] [NPE-691] Base pollsize to config --- lib/chore/queues/pubsub/consumer.rb | 9 +++++++-- spec/chore/queues/pubsub/consumer_spec.rb | 9 +++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/lib/chore/queues/pubsub/consumer.rb b/lib/chore/queues/pubsub/consumer.rb index 060d482..36b91a1 100644 --- a/lib/chore/queues/pubsub/consumer.rb +++ b/lib/chore/queues/pubsub/consumer.rb @@ -143,11 +143,16 @@ def pubsub end # Maximum number of messages to retrieve on each request from config - # Capped at 1000 messages which is the Pub/Sub limit + # Validates that the value doesn't exceed Pub/Sub's limit of 1000 messages # # @return [Integer] + # @raise [ArgumentError] if queue_polling_size exceeds 1000 def pubsub_polling_amount - [Chore.config.queue_polling_size, 1000].min + polling_size = Chore.config.queue_polling_size + if polling_size > 1000 + raise ArgumentError, "queue_polling_size (#{polling_size}) exceeds Google Cloud Pub/Sub maximum limit of 1000 messages" + end + polling_size end end end diff --git a/spec/chore/queues/pubsub/consumer_spec.rb b/spec/chore/queues/pubsub/consumer_spec.rb index b40e798..ad726ea 100644 --- a/spec/chore/queues/pubsub/consumer_spec.rb +++ b/spec/chore/queues/pubsub/consumer_spec.rb @@ -60,11 +60,16 @@ def consume(&block) consume end - it 'should respect Pub/Sub max limit of 1000 messages' do - allow(Chore.config).to receive(:queue_polling_size).and_return(2000) + it 'should allow exactly 1000 messages (Pub/Sub maximum)' do + allow(Chore.config).to receive(:queue_polling_size).and_return(1000) expect(subscription).to receive(:pull).with(max: 1000) consume end + + it 'should raise error when queue_polling_size exceeds Pub/Sub max limit of 1000 messages' do + allow(Chore.config).to receive(:queue_polling_size).and_return(2000) + expect { consumer.send(:pubsub_polling_amount) }.to raise_error(ArgumentError, /queue_polling_size \(2000\) exceeds Google Cloud Pub\/Sub maximum limit of 1000 messages/) + end end context 'with no messages' do From c7b651ce9345846f873872bff1e2ee97ded8d27a Mon Sep 17 00:00:00 2001 From: James Hong Date: Wed, 3 Sep 2025 16:58:55 -0700 Subject: [PATCH 17/26] [NPE-691] Update README for clarity regarding pubsub --- README.md | 76 +++++-------------------------------------------------- 1 file changed, 7 insertions(+), 69 deletions(-) diff --git a/README.md b/README.md index e41e118..127e06e 100644 --- a/README.md +++ b/README.md @@ -53,57 +53,11 @@ dependencies and required code. `AWS_SECRET_ACCESS_KEY` environment variables) and an AWS region is set (e.g. `AWS_REGION` environment variable) so that Chore can authenticate with AWS. - To use GCP Pub/Sub with Chore, you have several configuration options: - - **Option 1: Basic Setup** - Configure Chore to use the PubSub consumer and publisher: - - ```ruby - Chore.configure do |c| - c.consumer = Chore::Queues::PubSub::Consumer - c.publisher = Chore::Queues::PubSub::Publisher - end - ``` - - **Note**: Setting the consumer does not automatically set the publisher. You must configure both separately. - - **Note**: This configuration will use Google Cloud's automatic credential discovery (environment variables, service account, etc.). See the [Google Cloud PubSub Authentication documentation](https://cloud.google.com/ruby/docs/reference/google-cloud-pubsub/latest/AUTHENTICATION) for details. - - **Option 2: Global Configuration** - Set project and credentials that apply to all PubSub operations: - - ```ruby - Chore::Queues::PubSub.configure do |config| - config.project_id = 'your-project-id' - config.credentials = '/path/to/key.json' - end - ``` - - **Note**: This is equivalent to using the command line options `--gcp-project-id` and `--gcp-credentials`. See [Command Line Options](#command-line-options) for details. - - **Option 3: Per-Job Publisher Override** - Override the global publisher for specific jobs using `queue_options`: - - ```ruby - class MyJob - include Chore::Job - queue_options( - name: 'my_queue', - publisher: Chore::Queues::PubSub::Publisher - ) - - def perform(args={}) - # Job logic here - end - end - ``` - - **Note**: Publisher configuration (project_id, credentials) is set globally via environment variables or the `Chore::Queues::PubSub.configure` block. Individual jobs cannot override these settings. - +1. When using Google Pubsub: Chore will use Google Cloud's automatic credential discovery (environment variables, service account, etc.). See the [Google Cloud PubSub Authentication documentation](https://cloud.google.com/ruby/docs/reference/google-cloud-pubsub/latest/AUTHENTICATION) for details. Otherwise, you can configure Google::Pubsub directly. **Note**: PubSub requires gRPC which can have threading issues in some environments. google-cloud-pubsub 3.0+ includes improved gRPC handling, but if using PubSub in a threaded environment (such as with threaded consumer strategies), ensure you use gRPC version 1.74.1 or higher to avoid potential deadlocks and connection issues. - **Warning**: Using different queue types for consumer and publisher (e.g., PubSub consumer with SQS publisher) can be confusing and is generally not recommended. Mixed configurations should only be used carefully for specific scenarios like migrations, validations, or no-op queues. +1. Queue configuration **Configuration Priority**: Global configuration → Environment variables By default, Chore will run over all queues it detects among the required files. If different behavior is desired, @@ -118,25 +72,6 @@ Chore can authenticate with AWS. 1. Chore has many more options, which can be viewed by executing `bundle exec chore --help` -### Command Line Options - -In addition to configuration blocks, Chore supports command line options for queue-specific settings: - -**AWS SQS Options:** -``` ---aws-access-key KEY Valid AWS Access Key ---aws-secret-key KEY Valid AWS Secret Key ---dedupe-servers SERVERS List of memcache compatible server(s) for SQS message deduplication -``` - -**GCP Pub/Sub Options:** -``` ---gcp-project-id PROJECT_ID GCP Project ID for Pub/Sub ---gcp-credentials PATH Path to GCP service account credentials JSON file -``` - -These command line options set global defaults and are equivalent to setting the corresponding configuration in your code. - ### Tips For Configuring Chore For Rails, it can be necessary to add the jobs directory to the eager loading path, found in `application.rb`. A similar @@ -286,8 +221,6 @@ Chore provides the following built-in publishers: * `Chore::Queues::PubSub::Publisher` - For Google Cloud Pub/Sub * `Chore::Queues::Filesystem::Publisher` - For filesystem-based queues -**Note**: When using the `Chore::Queues::PubSub::Publisher`, you can configure it per-job, set defaults, or rely on Google Cloud's automatic credential discovery as described in the [GCP Pub/Sub Configuration Example](#gcp-pubsub-configuration-example) section. - It is worth noting that any option that can be set via config file or command-line args can also be set in a configure block. @@ -304,6 +237,10 @@ class MyJob end ``` +**Note**: Setting the publisher does not automatically set the consumer. You must configure both separately if not using defaults and planning to run publisher and consumer concurrently. + +**Warning**: Using different queue types for consumer and publisher (e.g., PubSub consumer with SQS publisher) can be confusing and is generally not recommended. Mixed configurations should only be used carefully for specific scenarios like migrations, validations, or no-op queues. + ### Consumer Configuration Unlike publishers, **consumers are configured globally and cannot be set per-job**. Consumers operate at the process level, polling queues and dispatching work to workers. They handle all queues in the system. @@ -359,6 +296,7 @@ GCP Pub/Sub uses a topic and subscription model. When using Chore with Pub/Sub: * Message delays are handled using subscription-level ack deadline modification instead of SQS visibility timeouts * Messages are acknowledged using subscription-level acknowledgment instead of being deleted * Pull-based consumption is used with configurable batch sizes (up to 1000 messages) +* Delivery attempt is not guaranteed. Generally, this is not an issue and will default to queue configurations for redrive policy, backoff, etc. ## Hooks From d83a867564e48aea1da33dfd800795846e7236b6 Mon Sep 17 00:00:00 2001 From: James Hong Date: Mon, 8 Sep 2025 16:43:48 -0700 Subject: [PATCH 18/26] [NPE-691] Clean up redundant spec --- spec/chore/queues/pubsub_spec.rb | 81 +++++++++----------------------- 1 file changed, 22 insertions(+), 59 deletions(-) diff --git a/spec/chore/queues/pubsub_spec.rb b/spec/chore/queues/pubsub_spec.rb index ec319db..5b26ef4 100644 --- a/spec/chore/queues/pubsub_spec.rb +++ b/spec/chore/queues/pubsub_spec.rb @@ -3,53 +3,44 @@ describe Chore::Queues::PubSub do include_context 'fake pubsub objects' + let(:topic_admin) { double('topic_admin') } + let(:subscription_admin) { double('subscription_admin') } + let(:topic_path) { "projects/test/topics/#{queue_name}" } + let(:subscription_path) { "projects/test/subscriptions/#{subscription_name}" } + before(:each) do allow(Chore::Queues::PubSub).to receive(:pubsub_client).and_return(pubsub_client) allow(Chore).to receive(:prefixed_queue_names).and_return([queue_name]) end context "when managing queues" do - it 'should create topics and subscriptions that are defined in its internal job name list' do - topic_admin = double('topic_admin') - subscription_admin = double('subscription_admin') + before(:each) do allow(pubsub_client).to receive(:topic_admin).and_return(topic_admin) allow(pubsub_client).to receive(:subscription_admin).and_return(subscription_admin) - allow(pubsub_client).to receive(:topic_path).with(queue_name).and_return("projects/test/topics/#{queue_name}") - allow(pubsub_client).to receive(:subscription_path).with(subscription_name).and_return("projects/test/subscriptions/#{subscription_name}") - expect(topic_admin).to receive(:create_topic).with(name: "projects/test/topics/#{queue_name}").and_return(topic) + allow(pubsub_client).to receive(:topic_path).with(queue_name).and_return(topic_path) + allow(pubsub_client).to receive(:subscription_path).with(subscription_name).and_return(subscription_path) + end + it 'should create topics and subscriptions that are defined in its internal job name list' do + expect(topic_admin).to receive(:create_topic).with(name: topic_path).and_return(topic) expect(subscription_admin).to receive(:create_subscription).with( - name: "projects/test/subscriptions/#{subscription_name}", - topic: "projects/test/topics/#{queue_name}" + name: subscription_path, + topic: topic_path ) Chore::Queues::PubSub.create_queues! end it 'should delete topics and subscriptions that are defined in its internal job name list' do - topic_admin = double('topic_admin') - subscription_admin = double('subscription_admin') - allow(pubsub_client).to receive(:topic_admin).and_return(topic_admin) - allow(pubsub_client).to receive(:subscription_admin).and_return(subscription_admin) - allow(pubsub_client).to receive(:topic_path).with(queue_name).and_return("projects/test/topics/#{queue_name}") - allow(pubsub_client).to receive(:subscription_path).with(subscription_name).and_return("projects/test/subscriptions/#{subscription_name}") - expect(Chore.logger).to receive(:info).with("Chore Deleting Pub/Sub Topic and Subscription: #{queue_name}") - expect(subscription_admin).to receive(:delete_subscription).with(subscription: "projects/test/subscriptions/#{subscription_name}") - expect(topic_admin).to receive(:delete_topic).with(topic: "projects/test/topics/#{queue_name}") + expect(subscription_admin).to receive(:delete_subscription).with(subscription: subscription_path) + expect(topic_admin).to receive(:delete_topic).with(topic: topic_path) Chore::Queues::PubSub.delete_queues! end context 'when handling delete errors' do it 'should continue deleting topic even if subscription delete fails' do - topic_admin = double('topic_admin') - subscription_admin = double('subscription_admin') - allow(pubsub_client).to receive(:topic_admin).and_return(topic_admin) - allow(pubsub_client).to receive(:subscription_admin).and_return(subscription_admin) - allow(pubsub_client).to receive(:topic_path).with(queue_name).and_return("projects/test/topics/#{queue_name}") - allow(pubsub_client).to receive(:subscription_path).with(subscription_name).and_return("projects/test/subscriptions/#{subscription_name}") - allow(subscription_admin).to receive(:delete_subscription).and_raise(Google::Cloud::NotFoundError.new('Subscription not found')) - expect(topic_admin).to receive(:delete_topic).with(topic: "projects/test/topics/#{queue_name}") + expect(topic_admin).to receive(:delete_topic).with(topic: topic_path) expect(Chore.logger).to receive(:info).with("Chore Deleting Pub/Sub Topic and Subscription: #{queue_name}") expect(Chore.logger).to receive(:error).with("Deleting Subscription: #{queue_name} failed because Subscription not found") @@ -57,14 +48,7 @@ end it 'should continue even if topic delete fails' do - topic_admin = double('topic_admin') - subscription_admin = double('subscription_admin') - allow(pubsub_client).to receive(:topic_admin).and_return(topic_admin) - allow(pubsub_client).to receive(:subscription_admin).and_return(subscription_admin) - allow(pubsub_client).to receive(:topic_path).with(queue_name).and_return("projects/test/topics/#{queue_name}") - allow(pubsub_client).to receive(:subscription_path).with(subscription_name).and_return("projects/test/subscriptions/#{subscription_name}") - - expect(subscription_admin).to receive(:delete_subscription).with(subscription: "projects/test/subscriptions/#{subscription_name}") + expect(subscription_admin).to receive(:delete_subscription).with(subscription: subscription_path) allow(topic_admin).to receive(:delete_topic).and_raise(Google::Cloud::NotFoundError.new('Topic not found')) expect(Chore.logger).to receive(:info).with("Chore Deleting Pub/Sub Topic and Subscription: #{queue_name}") expect(Chore.logger).to receive(:error).with("Deleting Topic: #{queue_name} failed because Topic not found") @@ -75,9 +59,12 @@ context 'and checking for existing queues' do it 'checks for existing topics/subscriptions' do - allow(pubsub_client).to receive(:topic_path).with(queue_name).and_return("projects/test/topics/#{queue_name}") - allow(pubsub_client).to receive(:subscription_path).with(subscription_name).and_return("projects/test/subscriptions/#{subscription_name}") + allow(pubsub_client).to receive(:topic_path).with(queue_name).and_return(topic_path) + allow(pubsub_client).to receive(:subscription_path).with(subscription_name).and_return(subscription_path) expect(described_class).to receive(:existing_queues).and_return([]) + # Since existing_queues returns [], create_queues! will proceed to create topics/subscriptions + allow(topic_admin).to receive(:create_topic).and_return(topic) + allow(subscription_admin).to receive(:create_subscription) Chore::Queues::PubSub.create_queues!(true) end @@ -87,12 +74,6 @@ end it 'does not raise an error if a topic/subscription does not exist' do - topic_admin = double('topic_admin') - subscription_admin = double('subscription_admin') - allow(pubsub_client).to receive(:topic_admin).and_return(topic_admin) - allow(pubsub_client).to receive(:subscription_admin).and_return(subscription_admin) - allow(pubsub_client).to receive(:topic_path).with(queue_name).and_return("projects/test/topics/#{queue_name}") - allow(pubsub_client).to receive(:subscription_path).with(subscription_name).and_return("projects/test/subscriptions/#{subscription_name}") allow(topic_admin).to receive(:create_topic).and_return(topic) allow(subscription_admin).to receive(:create_subscription) allow(described_class).to receive(:existing_queues).and_return([]) @@ -102,12 +83,6 @@ context 'when handling already exists errors' do it 'should continue when topic already exists' do - topic_admin = double('topic_admin') - subscription_admin = double('subscription_admin') - allow(pubsub_client).to receive(:topic_admin).and_return(topic_admin) - allow(pubsub_client).to receive(:subscription_admin).and_return(subscription_admin) - allow(pubsub_client).to receive(:topic_path).with(queue_name).and_return("projects/test/topics/#{queue_name}") - allow(pubsub_client).to receive(:subscription_path).with(subscription_name).and_return("projects/test/subscriptions/#{subscription_name}") allow(topic_admin).to receive(:create_topic).and_raise(Google::Cloud::AlreadyExistsError.new('Topic already exists')) allow(subscription_admin).to receive(:create_subscription).and_return(subscription) expect(Chore.logger).to receive(:info).with("Chore Creating Pub/Sub Topic and Subscription: #{queue_name}") @@ -116,12 +91,6 @@ end it 'should continue when subscription already exists' do - topic_admin = double('topic_admin') - subscription_admin = double('subscription_admin') - allow(pubsub_client).to receive(:topic_admin).and_return(topic_admin) - allow(pubsub_client).to receive(:subscription_admin).and_return(subscription_admin) - allow(pubsub_client).to receive(:topic_path).with(queue_name).and_return("projects/test/topics/#{queue_name}") - allow(pubsub_client).to receive(:subscription_path).with(subscription_name).and_return("projects/test/subscriptions/#{subscription_name}") allow(topic_admin).to receive(:create_topic).and_return(topic) allow(subscription_admin).to receive(:create_subscription).and_raise(Google::Cloud::AlreadyExistsError.new('Subscription already exists')) expect(Chore.logger).to receive(:info).with("Chore Creating Pub/Sub Topic and Subscription: #{queue_name}") @@ -130,12 +99,6 @@ end it 'should continue when both topic and subscription already exist' do - topic_admin = double('topic_admin') - subscription_admin = double('subscription_admin') - allow(pubsub_client).to receive(:topic_admin).and_return(topic_admin) - allow(pubsub_client).to receive(:subscription_admin).and_return(subscription_admin) - allow(pubsub_client).to receive(:topic_path).with(queue_name).and_return("projects/test/topics/#{queue_name}") - allow(pubsub_client).to receive(:subscription_path).with(subscription_name).and_return("projects/test/subscriptions/#{subscription_name}") allow(topic_admin).to receive(:create_topic).and_raise(Google::Cloud::AlreadyExistsError.new('Topic already exists')) allow(subscription_admin).to receive(:create_subscription).and_raise(Google::Cloud::AlreadyExistsError.new('Subscription already exists')) expect(Chore.logger).to receive(:info).with("Chore Creating Pub/Sub Topic and Subscription: #{queue_name}") From d067715251a82aacebdb15859baf37026033901e Mon Sep 17 00:00:00 2001 From: James Hong Date: Mon, 8 Sep 2025 16:44:56 -0700 Subject: [PATCH 19/26] [NPE-691] Remove errorneous version limit --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 127e06e..937f60a 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,7 @@ Chore can be integrated with any Ruby-based project by following these instructi For **Google Cloud Pub/Sub** (requires Ruby 3.1+ and google-cloud-pubsub 3.0+): ```ruby - gem 'google-cloud-pubsub', '>= 3.0', '< 4.0' + gem 'google-cloud-pubsub', '>= 3.0' ``` For **Filesystem queues**: No additional gems required. From e92363de5ac91f9858b0fbaaa77a3a95df9553b3 Mon Sep 17 00:00:00 2001 From: James Hong Date: Mon, 8 Sep 2025 17:06:31 -0700 Subject: [PATCH 20/26] [NPE-691] Update names to match --- lib/chore/queues/pubsub/consumer.rb | 36 ++++++++-------- lib/chore/queues/pubsub/publisher.rb | 20 ++++----- lib/chore/queues/sqs/consumer.rb | 2 - spec/chore/queues/pubsub/consumer_spec.rb | 48 +++++++++++----------- spec/chore/queues/pubsub/publisher_spec.rb | 40 +++++++++--------- spec/chore/queues/pubsub_spec.rb | 4 +- spec/support/queues/pubsub/fake_objects.rb | 22 +++++++--- 7 files changed, 90 insertions(+), 82 deletions(-) diff --git a/lib/chore/queues/pubsub/consumer.rb b/lib/chore/queues/pubsub/consumer.rb index 36b91a1..ec74bc3 100644 --- a/lib/chore/queues/pubsub/consumer.rb +++ b/lib/chore/queues/pubsub/consumer.rb @@ -7,6 +7,8 @@ module PubSub # GCP Pub/Sub Consumer for Chore. Requests messages from GCP Pub/Sub and passes them to be worked on. # Also controls acknowledging completed messages within GCP Pub/Sub. class Consumer < Chore::Consumer + MAX_PUBSUB_POLLING_SIZE = 1000.freeze + DEFAULT_DEADLINE_SECONDS = 600.freeze # Initialize the reset at on class load @@reset_at = Time.now @@ -24,7 +26,7 @@ def initialize(queue_name, opts={}) # Ensure that the consumer is capable of running def verify_connection! - unless subscription.exists? + unless subscriber.exists? raise "Subscription #{@subscription_name} does not exist" end end @@ -32,8 +34,6 @@ def verify_connection! # Begins requesting messages from GCP Pub/Sub, which will invoke the +&handler+ over each message # # @param [Block] &handler Message handler, used by the calling context (worker) to create & assigns a UnitOfWork - # - # @return [Array] def consume(&handler) while running? begin @@ -54,14 +54,14 @@ def reject(message_id) # In Pub/Sub, we can simply not acknowledge the message and it will be redelivered end - # Acknowledges the given message from the GCP Pub/Sub subscription + # Acknowledges the given message from the GCP Pub/Sub subscriber # # @param [String] message_id Unique ID of the Pub/Sub message # @param [String] ack_id Acknowledgment ID of the Pub/Sub message def complete(message_id, ack_id) Chore.logger.debug "Completing (acknowledging): #{message_id} ack_id: #{ack_id}" # Find the message by ack_id and acknowledge it - subscription.acknowledge(ack_id) + subscriber.acknowledge(ack_id) end # Delays retry of a job by +backoff_calc+ seconds. @@ -74,7 +74,7 @@ def delay(item, backoff_calc) Chore.logger.debug "Delaying #{item.id} by #{delay} seconds" # Find the message and modify its ack deadline - subscription.modify_ack_deadline(delay, item.receipt_handle) + subscriber.modify_ack_deadline(delay, item.receipt_handle) return delay end @@ -95,7 +95,7 @@ def handle_messages(&block) raise Chore::TerribleMistake end - messages = subscription.pull(max: pubsub_polling_amount) + messages = subscriber.pull(max: pubsub_polling_amount) received_timestamp = Time.now messages.each do |message| @@ -112,27 +112,27 @@ def handle_messages(&block) messages end - # Retrieves the GCP Pub/Sub subscription object. The method will cache the results to prevent round trips on subsequent calls + # Retrieves the GCP Pub/Sub subscriber object. The method will cache the results to prevent round trips on subsequent calls # # If reset_connection! has been called, this will result in the connection being re-initialized, # as well as clear any cached results from prior calls # - # @return [Google::Cloud::PubSub::Subscription] - def subscription + # @return [Google::Cloud::PubSub::Subscriber] + def subscriber if !@pubsub_last_connected || (@@reset_at && @@reset_at >= @pubsub_last_connected) @pubsub = nil @pubsub_last_connected = Time.now - @subscription = nil + @subscriber = nil end - @subscription ||= pubsub.subscriber(@subscription_name) + @subscriber ||= pubsub.subscriber(@subscription_name) end - # The ack deadline (in seconds) of the subscription + # The ack deadline (in seconds) of the subscriber # # @return [Integer] def queue_timeout - @queue_timeout ||= subscription.deadline || 600 # Default to 10 minutes + @queue_timeout ||= subscriber.deadline || DEFAULT_DEADLINE_SECONDS # Default to 10 minutes end # GCP Pub/Sub client object @@ -143,14 +143,14 @@ def pubsub end # Maximum number of messages to retrieve on each request from config - # Validates that the value doesn't exceed Pub/Sub's limit of 1000 messages + # Validates that the value doesn't exceed Pub/Sub's limit of MAX_PUBSUB_POLLING_SIZE messages # # @return [Integer] - # @raise [ArgumentError] if queue_polling_size exceeds 1000 + # @raise [Chore::TerribleMistake] if queue_polling_size exceeds MAX_PUBSUB_POLLING_SIZE def pubsub_polling_amount polling_size = Chore.config.queue_polling_size - if polling_size > 1000 - raise ArgumentError, "queue_polling_size (#{polling_size}) exceeds Google Cloud Pub/Sub maximum limit of 1000 messages" + if polling_size > MAX_PUBSUB_POLLING_SIZE + raise Chore::TerribleMistake, "queue_polling_size (#{polling_size}) exceeds Google Cloud Pub/Sub maximum limit of #{MAX_PUBSUB_POLLING_SIZE} messages" end polling_size end diff --git a/lib/chore/queues/pubsub/publisher.rb b/lib/chore/queues/pubsub/publisher.rb index 7bf34c5..e17625a 100644 --- a/lib/chore/queues/pubsub/publisher.rb +++ b/lib/chore/queues/pubsub/publisher.rb @@ -10,7 +10,7 @@ class Publisher < Chore::Publisher # @param [Hash] opts Publisher options def initialize(opts={}) super - @pubsub_topics = {} + @pubsub_publisher = {} end # Publishes a message to a GCP Pub/Sub topic @@ -20,9 +20,9 @@ def initialize(opts={}) # # @return [Google::Cloud::PubSub::Message] def publish(queue_name, job) - topic = get_topic(queue_name) + publisher = get_publisher(queue_name) encoded_job = encode_job(job) - topic.publish(encoded_job) + publisher.publish(encoded_job) end # Sets a flag that instructs the publisher to reset the connection the next time it's used @@ -39,24 +39,24 @@ def pubsub @pubsub ||= Chore::Queues::PubSub.pubsub_client end - # Retrieves the GCP Pub/Sub topic object. The method will cache the results to prevent round trips on subsequent calls + # Retrieves the GCP Pub/Sub publisher object. The method will cache the results to prevent round trips on subsequent calls # # If reset_connection! has been called, this will result in the connection being re-initialized, # as well as clear any cached results from prior calls # - # @param [String] name Name of GCP Pub/Sub topic + # @param [String] name Name of GCP Pub/Sub topic # - # @return [Google::Cloud::PubSub::Topic] - def get_topic(name) + # @return [Google::Cloud::PubSub::Publisher] + def get_publisher(name) if @@reset_next @pubsub = nil @@reset_next = false - @pubsub_topics = {} + @pubsub_publisher = {} end - @pubsub_topics[name] ||= pubsub.publisher(name) + @pubsub_publisher[name] ||= pubsub.publisher(name) end end end end -end \ No newline at end of file +end diff --git a/lib/chore/queues/sqs/consumer.rb b/lib/chore/queues/sqs/consumer.rb index 3834e72..33084b9 100644 --- a/lib/chore/queues/sqs/consumer.rb +++ b/lib/chore/queues/sqs/consumer.rb @@ -34,8 +34,6 @@ def verify_connection! # Begins requesting messages from SQS, which will invoke the +&handler+ over each message # # @param [Block] &handler Message handler, used by the calling context (worker) to create & assigns a UnitOfWork - # - # @return [Array] def consume(&handler) while running? begin diff --git a/spec/chore/queues/pubsub/consumer_spec.rb b/spec/chore/queues/pubsub/consumer_spec.rb index ad726ea..7f3f07a 100644 --- a/spec/chore/queues/pubsub/consumer_spec.rb +++ b/spec/chore/queues/pubsub/consumer_spec.rb @@ -17,7 +17,7 @@ def consume(&block) before do allow(Chore::Queues::PubSub).to receive(:pubsub_client).and_return(pubsub_client) - allow(subscription).to receive(:pull).and_return([received_message]) + allow(subscriber).to receive(:pull).and_return([received_message]) end describe "consuming messages" do @@ -27,7 +27,7 @@ def consume(&block) context "should create objects for interacting with the Pub/Sub API" do it 'should create a pubsub client' do - expect(subscription).to receive(:pull) + expect(subscriber).to receive(:pull) consume end @@ -37,38 +37,38 @@ def consume(&block) consume end - it 'should look up the subscription based on the queue name' do + it 'should look up the subscriber based on the queue name' do expect(pubsub_client).to receive(:subscriber).with(subscription_name) consume end - it 'should create a subscription object' do - expect(consumer.send(:subscription)).to_not be_nil + it 'should create a subscriber object' do + expect(consumer.send(:subscriber)).to_not be_nil consume end end - context "should receive a message from the subscription" do + context "should receive a message from the subscriber" do it 'should use the default size of 10 when no queue_polling_size is specified' do - expect(subscription).to receive(:pull).with(max: 10).and_return([received_message]) + expect(subscriber).to receive(:pull).with(max: 10).and_return([received_message]) consume end it 'should respect the queue_polling_size when specified' do allow(Chore.config).to receive(:queue_polling_size).and_return(5) - expect(subscription).to receive(:pull).with(max: 5) + expect(subscriber).to receive(:pull).with(max: 5) consume end it 'should allow exactly 1000 messages (Pub/Sub maximum)' do allow(Chore.config).to receive(:queue_polling_size).and_return(1000) - expect(subscription).to receive(:pull).with(max: 1000) + expect(subscriber).to receive(:pull).with(max: 1000) consume end it 'should raise error when queue_polling_size exceeds Pub/Sub max limit of 1000 messages' do allow(Chore.config).to receive(:queue_polling_size).and_return(2000) - expect { consumer.send(:pubsub_polling_amount) }.to raise_error(ArgumentError, /queue_polling_size \(2000\) exceeds Google Cloud Pub\/Sub maximum limit of 1000 messages/) + expect { consumer.send(:pubsub_polling_amount) }.to raise_error(Chore::TerribleMistake, /queue_polling_size \(2000\) exceeds Google Cloud Pub\/Sub maximum limit of 1000 messages/) end end @@ -86,7 +86,7 @@ def consume(&block) context 'with messages' do before do allow(consumer).to receive(:duplicate_message?).and_return(false) - allow(subscription).to receive(:pull).and_return([received_message]) + allow(subscriber).to receive(:pull).and_return([received_message]) allow(Time).to receive(:now).and_return(Time.utc(2024, 5, 10, 12, 0, 0)) end @@ -103,7 +103,7 @@ def consume(&block) received_message.message_id, received_message.ack_id, queue_name, - subscription.deadline, + subscriber.deadline, received_message.data, 0, # delivery_attempt - 1 (1 - 1 = 0) received_timestamp @@ -136,7 +136,7 @@ def consume(&block) received_message.message_id, received_message.ack_id, queue_name, - subscription.deadline, + subscriber.deadline, received_message.data, 2, # delivery_attempt - 1 (3 - 1 = 2) received_timestamp @@ -155,7 +155,7 @@ def consume(&block) received_message.message_id, received_message.ack_id, queue_name, - subscription.deadline, + subscriber.deadline, received_message.data, 0, # (nil || 1) - 1 = 0 received_timestamp @@ -164,7 +164,7 @@ def consume(&block) end end - context 'on subscription lookup failure' do + context 'on subscriber lookup failure' do before(:each) do allow(consumer).to receive(:verify_connection!).and_raise(Google::Cloud::NotFoundError.new('Subscription not found')) end @@ -198,8 +198,8 @@ def consume(&block) end describe "completing work" do - it 'acknowledges the message in the subscription' do - expect(subscription).to receive(:acknowledge).with(received_message.ack_id) + it 'acknowledges the message in the subscriber' do + expect(subscriber).to receive(:acknowledge).with(received_message.ack_id) consumer.complete(received_message.message_id, received_message.ack_id) end @@ -213,7 +213,7 @@ def consume(&block) it 'modifies the ack deadline of the message' do delay_seconds = backoff_func.call(item) - expect(subscription).to receive(:modify_ack_deadline).with(delay_seconds, received_message.ack_id) + expect(subscriber).to receive(:modify_ack_deadline).with(delay_seconds, received_message.ack_id) consumer.delay(item, backoff_func) end @@ -271,24 +271,24 @@ def consume(&block) end describe '#verify_connection!' do - it 'should verify subscription exists' do - expect(subscription).to receive(:exists?).and_return(true) + it 'should verify subscriber exists' do + expect(subscriber).to receive(:exists?).and_return(true) expect { consumer.verify_connection! }.not_to raise_error end - it 'should raise error if subscription does not exist' do - allow(subscription).to receive(:exists?).and_return(false) + it 'should raise error if subscriber does not exist' do + allow(subscriber).to receive(:exists?).and_return(false) expect { consumer.verify_connection! }.to raise_error end end describe 'queue timeout' do - it 'should return subscription ack deadline' do + it 'should return subscriber ack deadline' do expect(consumer.send(:queue_timeout)).to eq(600) end it 'should default to 600 seconds if deadline is nil' do - allow(subscription).to receive(:deadline).and_return(nil) + allow(subscriber).to receive(:deadline).and_return(nil) expect(consumer.send(:queue_timeout)).to eq(600) end end diff --git a/spec/chore/queues/pubsub/publisher_spec.rb b/spec/chore/queues/pubsub/publisher_spec.rb index 9364ea5..d67b63f 100644 --- a/spec/chore/queues/pubsub/publisher_spec.rb +++ b/spec/chore/queues/pubsub/publisher_spec.rb @@ -3,52 +3,49 @@ describe Chore::Queues::PubSub::Publisher do include_context 'fake pubsub objects' - let(:publisher) { Chore::Queues::PubSub::Publisher.new } + let(:spec_publisher) { Chore::Queues::PubSub::Publisher.new } let(:job) { {'class' => 'TestJob', 'args'=>[1,2,'3']} } let(:publish_result) { double('Google::Cloud::PubSub::Message', message_id: message_id) } before(:each) do allow(Chore::Queues::PubSub).to receive(:pubsub_client).and_return(pubsub_client) - allow(topic).to receive(:publish).and_return(publish_result) + allow(publisher).to receive(:publish).and_return(publish_result) end it 'should configure pubsub client' do expect(Chore::Queues::PubSub).to receive(:pubsub_client) - publisher.publish(queue_name, job) + spec_publisher.publish(queue_name, job) end it 'should not create a new Pub/Sub client before every publish' do expect(Chore::Queues::PubSub).to receive(:pubsub_client).once - 2.times { publisher.send(:get_topic, queue_name) } + 2.times { spec_publisher.send(:get_publisher, queue_name) } end it 'should lookup the topic when publishing' do - expect(pubsub_client).to receive(:publisher).with(queue_name).and_return(topic) - publisher.publish(queue_name, job) + expect(pubsub_client).to receive(:publisher).with(queue_name).and_return(publisher) + spec_publisher.publish(queue_name, job) end it 'should publish an encoded message to the specified topic' do - expect(topic).to receive(:publish).with(job.to_json) - publisher.publish(queue_name, job) + expect(publisher).to receive(:publish).with(job.to_json) + spec_publisher.publish(queue_name, job) end it 'should lookup multiple topics if specified' do second_queue_name = queue_name + '2' - second_topic = double('Google::Cloud::PubSub::Topic') + second_publisher = double('Google::Cloud::PubSub::Publisher') - expect(pubsub_client).to receive(:publisher).with(queue_name).and_return(topic) - expect(pubsub_client).to receive(:publisher).with(second_queue_name).and_return(second_topic) - expect(topic).to receive(:publish) - expect(second_topic).to receive(:publish) + expect(pubsub_client).to receive(:publisher).with(queue_name).and_return(publisher) + expect(pubsub_client).to receive(:publisher).with(second_queue_name).and_return(second_publisher) + expect(publisher).to receive(:publish) + expect(second_publisher).to receive(:publish) - publisher.publish(queue_name, job) - publisher.publish(second_queue_name, job) + spec_publisher.publish(queue_name, job) + spec_publisher.publish(second_queue_name, job) end it 'should only lookup a named topic once' do - expect(pubsub_client).to receive(:publisher).with(queue_name).once.and_return(topic) - expect(topic).to receive(:publish).exactly(4).times - 4.times { publisher.publish(queue_name, job) } end describe '#reset_connection!' do @@ -80,12 +77,15 @@ Chore::Queues::PubSub::Publisher.reset_connection! 4.times { publisher.send(:get_topic, queue_name) } end + expect(pubsub_client).to receive(:publisher).with(queue_name).once.and_return(publisher) + expect(publisher).to receive(:publish).exactly(4).times + 4.times { spec_publisher.publish(queue_name, job) } end describe 'encoding' do it 'should encode job to JSON' do - encoded_job = publisher.send(:encode_job, job) + encoded_job = spec_publisher.send(:encode_job, job) expect(encoded_job).to eq(job.to_json) end end -end \ No newline at end of file +end diff --git a/spec/chore/queues/pubsub_spec.rb b/spec/chore/queues/pubsub_spec.rb index 5b26ef4..e5bb653 100644 --- a/spec/chore/queues/pubsub_spec.rb +++ b/spec/chore/queues/pubsub_spec.rb @@ -84,7 +84,7 @@ context 'when handling already exists errors' do it 'should continue when topic already exists' do allow(topic_admin).to receive(:create_topic).and_raise(Google::Cloud::AlreadyExistsError.new('Topic already exists')) - allow(subscription_admin).to receive(:create_subscription).and_return(subscription) + allow(subscription_admin).to receive(:create_subscription).and_return(subscriber) expect(Chore.logger).to receive(:info).with("Chore Creating Pub/Sub Topic and Subscription: #{queue_name}") expect(Chore.logger).to receive(:info).with("Topic already exists: Topic already exists") expect { Chore::Queues::PubSub.create_queues! }.not_to raise_error @@ -112,7 +112,7 @@ describe '.existing_queues' do it 'returns queues that exist when both publisher and subscriber calls succeed' do allow(pubsub_client).to receive(:publisher).with(queue_name).and_return(topic) - allow(pubsub_client).to receive(:subscriber).with(queue_name).and_return(subscription) + allow(pubsub_client).to receive(:subscriber).with(queue_name).and_return(subscriber) expect(described_class.existing_queues).to eq([queue_name]) end diff --git a/spec/support/queues/pubsub/fake_objects.rb b/spec/support/queues/pubsub/fake_objects.rb index 9a91649..7a08461 100644 --- a/spec/support/queues/pubsub/fake_objects.rb +++ b/spec/support/queues/pubsub/fake_objects.rb @@ -22,13 +22,23 @@ name: queue_name, exists?: true, publish: received_message, - create_subscription: subscription, + create_subscription: subscriber, delete: true ) end - let(:subscription) do - double('Google::Cloud::PubSub::Subscription', + let(:publisher) do + double('Google::Cloud::PubSub::Publisher', + name: queue_name, + exists?: true, + publish: received_message, + create_subscription: subscriber, + delete: true + ) + end + + let(:subscriber) do + double('Google::Cloud::PubSub::Subscriber', name: subscription_name, exists?: true, deadline: 600, @@ -45,14 +55,14 @@ delete_topic: true ) subscription_admin = double('subscription_admin', - create_subscription: subscription, + create_subscription: subscriber, delete_subscription: true ) double('Google::Cloud::PubSub::Project', project_id: project_id, - publisher: topic, - subscriber: subscription, + publisher: publisher, + subscriber: subscriber, topic_admin: topic_admin, subscription_admin: subscription_admin, topic_path: "projects/#{project_id}/topics/#{queue_name}", From 136863b46e20eea1758e5bc569e226c849a92dad Mon Sep 17 00:00:00 2001 From: James Hong Date: Mon, 8 Sep 2025 17:34:23 -0700 Subject: [PATCH 21/26] [NPE-691] Make Pubsub handle connections --- lib/chore/queues/pubsub/consumer.rb | 16 ---------- lib/chore/queues/pubsub/publisher.rb | 15 ---------- spec/chore/queues/pubsub/consumer_spec.rb | 34 ---------------------- spec/chore/queues/pubsub/publisher_spec.rb | 31 -------------------- 4 files changed, 96 deletions(-) diff --git a/lib/chore/queues/pubsub/consumer.rb b/lib/chore/queues/pubsub/consumer.rb index ec74bc3..b1f0484 100644 --- a/lib/chore/queues/pubsub/consumer.rb +++ b/lib/chore/queues/pubsub/consumer.rb @@ -9,13 +9,6 @@ module PubSub class Consumer < Chore::Consumer MAX_PUBSUB_POLLING_SIZE = 1000.freeze DEFAULT_DEADLINE_SECONDS = 600.freeze - # Initialize the reset at on class load - @@reset_at = Time.now - - # Resets the API client connection and provides @@reset_at so we know when the last time that was done - def self.reset_connection! - @@reset_at = Time.now - end # @param [String] queue_name Name of GCP Pub/Sub topic # @param [Hash] opts Options @@ -114,17 +107,8 @@ def handle_messages(&block) # Retrieves the GCP Pub/Sub subscriber object. The method will cache the results to prevent round trips on subsequent calls # - # If reset_connection! has been called, this will result in the connection being re-initialized, - # as well as clear any cached results from prior calls - # # @return [Google::Cloud::PubSub::Subscriber] def subscriber - if !@pubsub_last_connected || (@@reset_at && @@reset_at >= @pubsub_last_connected) - @pubsub = nil - @pubsub_last_connected = Time.now - @subscriber = nil - end - @subscriber ||= pubsub.subscriber(@subscription_name) end diff --git a/lib/chore/queues/pubsub/publisher.rb b/lib/chore/queues/pubsub/publisher.rb index e17625a..11293ea 100644 --- a/lib/chore/queues/pubsub/publisher.rb +++ b/lib/chore/queues/pubsub/publisher.rb @@ -5,8 +5,6 @@ module Queues module PubSub # GCP Pub/Sub Publisher, for writing messages to GCP Pub/Sub from Chore class Publisher < Chore::Publisher - @@reset_next = true - # @param [Hash] opts Publisher options def initialize(opts={}) super @@ -25,10 +23,6 @@ def publish(queue_name, job) publisher.publish(encoded_job) end - # Sets a flag that instructs the publisher to reset the connection the next time it's used - def self.reset_connection! - @@reset_next = true - end private @@ -41,19 +35,10 @@ def pubsub # Retrieves the GCP Pub/Sub publisher object. The method will cache the results to prevent round trips on subsequent calls # - # If reset_connection! has been called, this will result in the connection being re-initialized, - # as well as clear any cached results from prior calls - # # @param [String] name Name of GCP Pub/Sub topic # # @return [Google::Cloud::PubSub::Publisher] def get_publisher(name) - if @@reset_next - @pubsub = nil - @@reset_next = false - @pubsub_publisher = {} - end - @pubsub_publisher[name] ||= pubsub.publisher(name) end end diff --git a/spec/chore/queues/pubsub/consumer_spec.rb b/spec/chore/queues/pubsub/consumer_spec.rb index 7f3f07a..057684c 100644 --- a/spec/chore/queues/pubsub/consumer_spec.rb +++ b/spec/chore/queues/pubsub/consumer_spec.rb @@ -235,40 +235,6 @@ def consume(&block) end end - describe '#reset_connection!' do - it 'should reset the connection after a call to reset_connection!' do - # First call establishes the connection - consumer.send(:subscription) - - # Reset connection - Chore::Queues::PubSub::Consumer.reset_connection! - - # Should recreate client - expect(Chore::Queues::PubSub).to receive(:pubsub_client).and_return(pubsub_client) - consumer.send(:subscription) - end - - it 'should not reset the connection between calls' do - expect(Chore::Queues::PubSub).to receive(:pubsub_client).once.and_return(pubsub_client) - s = consumer.send(:subscription) - expect(consumer.send(:subscription)).to be(s) - end - - it 'should reconfigure pubsub client' do - allow(consumer).to receive(:running?).and_return(true, false) - allow_any_instance_of(Chore::DuplicateDetector).to receive(:found_duplicate?).and_return(false) - - allow(subscription).to receive(:pull).and_return([received_message]) - - consume - - Chore::Queues::PubSub::Consumer.reset_connection! - allow(Chore::Queues::PubSub).to receive(:pubsub_client).and_return(pubsub_client) - - expect(consumer).to receive(:running?).and_return(true, false) - consume - end - end describe '#verify_connection!' do it 'should verify subscriber exists' do diff --git a/spec/chore/queues/pubsub/publisher_spec.rb b/spec/chore/queues/pubsub/publisher_spec.rb index d67b63f..2208118 100644 --- a/spec/chore/queues/pubsub/publisher_spec.rb +++ b/spec/chore/queues/pubsub/publisher_spec.rb @@ -46,37 +46,6 @@ end it 'should only lookup a named topic once' do - end - - describe '#reset_connection!' do - it 'should reset client connection after a call to reset_connection!' do - # First call establishes the client - publisher.send(:get_topic, queue_name) - - # Reset and verify client is reset - Chore::Queues::PubSub::Publisher.reset_connection! - - expect(Chore::Queues::PubSub).to receive(:pubsub_client).and_return(pubsub_client) - publisher.send(:get_topic, queue_name) - end - - it 'should clear topic cache after reset_connection!' do - # Establish topic cache - publisher.send(:get_topic, queue_name) - - # Reset connection - Chore::Queues::PubSub::Publisher.reset_connection! - - # Should lookup topic again - expect(pubsub_client).to receive(:publisher).with(queue_name).and_return(topic) - publisher.send(:get_topic, queue_name) - end - - it 'should not reset the connection between calls' do - expect(Chore::Queues::PubSub).to receive(:pubsub_client).once.and_return(pubsub_client) - Chore::Queues::PubSub::Publisher.reset_connection! - 4.times { publisher.send(:get_topic, queue_name) } - end expect(pubsub_client).to receive(:publisher).with(queue_name).once.and_return(publisher) expect(publisher).to receive(:publish).exactly(4).times 4.times { spec_publisher.publish(queue_name, job) } From 90a6e40e884b22e2128f357d46cb17e5f9b88ac8 Mon Sep 17 00:00:00 2001 From: James Hong Date: Mon, 8 Sep 2025 17:38:58 -0700 Subject: [PATCH 22/26] [NPE-691] More spec cleanup --- spec/chore/queues/pubsub/consumer_spec.rb | 78 ++++++++--------------- 1 file changed, 25 insertions(+), 53 deletions(-) diff --git a/spec/chore/queues/pubsub/consumer_spec.rb b/spec/chore/queues/pubsub/consumer_spec.rb index 057684c..845f038 100644 --- a/spec/chore/queues/pubsub/consumer_spec.rb +++ b/spec/chore/queues/pubsub/consumer_spec.rb @@ -115,86 +115,58 @@ def consume(&block) consume end - context 'with duplicates' do - before do - allow(consumer).to receive(:duplicate_message?).and_return(true) - end - - it 'should not yield for a dupe message' do - expect {|b| consume(&b) }.not_to yield_control - end + it 'should not yield for a dupe message' do + allow(consumer).to receive(:duplicate_message?).and_return(true) + expect {|b| consume(&b) }.not_to yield_control end context 'with delivery attempt count' do - before do - allow(received_message).to receive(:delivery_attempt).and_return(3) - end - it 'should calculate attempt count as delivery_attempt - 1' do + allow(received_message).to receive(:delivery_attempt).and_return(3) expect { |b| consume(&b) } .to yield_with_args( - received_message.message_id, - received_message.ack_id, - queue_name, - subscriber.deadline, - received_message.data, - 2, # delivery_attempt - 1 (3 - 1 = 2) - received_timestamp - ) - end - end - - context 'with nil delivery attempt (older messages)' do - before do - allow(received_message).to receive(:delivery_attempt).and_return(nil) + received_message.message_id, + received_message.ack_id, + queue_name, + subscriber.deadline, + received_message.data, + 2, # delivery_attempt - 1 (3 - 1 = 2) + received_timestamp + ) end it 'should default to attempt count of 0' do + allow(received_message).to receive(:delivery_attempt).and_return(nil) expect { |b| consume(&b) } .to yield_with_args( - received_message.message_id, - received_message.ack_id, - queue_name, - subscriber.deadline, - received_message.data, - 0, # (nil || 1) - 1 = 0 - received_timestamp - ) + received_message.message_id, + received_message.ack_id, + queue_name, + subscriber.deadline, + received_message.data, + 0, # (nil || 1) - 1 = 0 + received_timestamp + ) end end end - context 'on subscriber lookup failure' do - before(:each) do + context "failure scenarios" do + it 'should raise exception on subscriber lookup failure' do allow(consumer).to receive(:verify_connection!).and_raise(Google::Cloud::NotFoundError.new('Subscription not found')) - end - - it 'should raise exception' do expect { consume }.to raise_error(Chore::TerribleMistake) end - end - context 'on gcp credential failure' do - before(:each) do + it 'should raise exception for gcp credential failure' do allow(consumer).to receive(:verify_connection!).and_raise(Google::Cloud::PermissionDeniedError.new('Permission denied')) - end - - it 'should raise exception' do expect { consume }.to raise_error(Chore::TerribleMistake) end - end - context 'on unexpected failure' do - before(:each) do + it 'should raise exception during unexpected failure' do allow(consumer).to receive(:verify_connection!).and_raise(StandardError.new('Connection error')) - end - - it 'should raise exception' do expect { consume }.to raise_error(Chore::TerribleMistake) end end - - end describe "completing work" do From 520d9af2cc5d10dde25bc357dafa8a2cec8b3b9a Mon Sep 17 00:00:00 2001 From: James Hong Date: Mon, 8 Sep 2025 17:44:18 -0700 Subject: [PATCH 23/26] [NPE-691] Rescue specific GCP error --- lib/chore/queues/pubsub.rb | 2 +- spec/chore/queues/pubsub_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/chore/queues/pubsub.rb b/lib/chore/queues/pubsub.rb index 13b24e9..de5c258 100644 --- a/lib/chore/queues/pubsub.rb +++ b/lib/chore/queues/pubsub.rb @@ -121,7 +121,7 @@ def self.existing_queues client.subscriber(queue_name) # if both publisher/subscriber successfully load, then assume exists true - rescue + rescue Google::Cloud::NotFoundError # google api throws Google::Cloud::NotFoundError if topic/subscription does not exist false end diff --git a/spec/chore/queues/pubsub_spec.rb b/spec/chore/queues/pubsub_spec.rb index e5bb653..03b12fe 100644 --- a/spec/chore/queues/pubsub_spec.rb +++ b/spec/chore/queues/pubsub_spec.rb @@ -133,7 +133,7 @@ end it 'handles other errors gracefully' do - allow(pubsub_client).to receive(:publisher).with(queue_name).and_raise(StandardError.new('Connection error')) + allow(pubsub_client).to receive(:publisher).with(queue_name).and_raise(Google::Cloud::NotFoundError.new('Connection error')) expect(described_class.existing_queues).to eq([]) end end From b16a6ec4b702eb159e95568d90ec0494b0e12b1a Mon Sep 17 00:00:00 2001 From: James Hong Date: Mon, 8 Sep 2025 17:44:34 -0700 Subject: [PATCH 24/26] [NPE-691] Remove logging specs --- spec/chore/queues/pubsub_spec.rb | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/spec/chore/queues/pubsub_spec.rb b/spec/chore/queues/pubsub_spec.rb index 03b12fe..7d88169 100644 --- a/spec/chore/queues/pubsub_spec.rb +++ b/spec/chore/queues/pubsub_spec.rb @@ -30,7 +30,6 @@ end it 'should delete topics and subscriptions that are defined in its internal job name list' do - expect(Chore.logger).to receive(:info).with("Chore Deleting Pub/Sub Topic and Subscription: #{queue_name}") expect(subscription_admin).to receive(:delete_subscription).with(subscription: subscription_path) expect(topic_admin).to receive(:delete_topic).with(topic: topic_path) @@ -41,7 +40,6 @@ it 'should continue deleting topic even if subscription delete fails' do allow(subscription_admin).to receive(:delete_subscription).and_raise(Google::Cloud::NotFoundError.new('Subscription not found')) expect(topic_admin).to receive(:delete_topic).with(topic: topic_path) - expect(Chore.logger).to receive(:info).with("Chore Deleting Pub/Sub Topic and Subscription: #{queue_name}") expect(Chore.logger).to receive(:error).with("Deleting Subscription: #{queue_name} failed because Subscription not found") Chore::Queues::PubSub.delete_queues! @@ -50,7 +48,6 @@ it 'should continue even if topic delete fails' do expect(subscription_admin).to receive(:delete_subscription).with(subscription: subscription_path) allow(topic_admin).to receive(:delete_topic).and_raise(Google::Cloud::NotFoundError.new('Topic not found')) - expect(Chore.logger).to receive(:info).with("Chore Deleting Pub/Sub Topic and Subscription: #{queue_name}") expect(Chore.logger).to receive(:error).with("Deleting Topic: #{queue_name} failed because Topic not found") Chore::Queues::PubSub.delete_queues! @@ -85,25 +82,18 @@ it 'should continue when topic already exists' do allow(topic_admin).to receive(:create_topic).and_raise(Google::Cloud::AlreadyExistsError.new('Topic already exists')) allow(subscription_admin).to receive(:create_subscription).and_return(subscriber) - expect(Chore.logger).to receive(:info).with("Chore Creating Pub/Sub Topic and Subscription: #{queue_name}") - expect(Chore.logger).to receive(:info).with("Topic already exists: Topic already exists") expect { Chore::Queues::PubSub.create_queues! }.not_to raise_error end it 'should continue when subscription already exists' do allow(topic_admin).to receive(:create_topic).and_return(topic) allow(subscription_admin).to receive(:create_subscription).and_raise(Google::Cloud::AlreadyExistsError.new('Subscription already exists')) - expect(Chore.logger).to receive(:info).with("Chore Creating Pub/Sub Topic and Subscription: #{queue_name}") - expect(Chore.logger).to receive(:info).with("Subscription already exists: Subscription already exists") expect { Chore::Queues::PubSub.create_queues! }.not_to raise_error end it 'should continue when both topic and subscription already exist' do allow(topic_admin).to receive(:create_topic).and_raise(Google::Cloud::AlreadyExistsError.new('Topic already exists')) allow(subscription_admin).to receive(:create_subscription).and_raise(Google::Cloud::AlreadyExistsError.new('Subscription already exists')) - expect(Chore.logger).to receive(:info).with("Chore Creating Pub/Sub Topic and Subscription: #{queue_name}") - expect(Chore.logger).to receive(:info).with("Topic already exists: Topic already exists") - expect(Chore.logger).to receive(:info).with("Subscription already exists: Subscription already exists") expect { Chore::Queues::PubSub.create_queues! }.not_to raise_error end end From 850df1904c0eb810e6966eb38c747149be092656 Mon Sep 17 00:00:00 2001 From: James Hong Date: Mon, 8 Sep 2025 17:44:48 -0700 Subject: [PATCH 25/26] [NPE-691] Simplify min version check --- lib/chore/queues/pubsub.rb | 13 ++++--------- lib/chore/queues/sqs.rb | 10 +++------- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/lib/chore/queues/pubsub.rb b/lib/chore/queues/pubsub.rb index de5c258..69be40e 100644 --- a/lib/chore/queues/pubsub.rb +++ b/lib/chore/queues/pubsub.rb @@ -9,17 +9,12 @@ module PubSub def self.pubsub_client require REQUIRED_LIBRARY - # Verify compatible version - begin - gem_version = Gem::Version.new(Google::Cloud::PubSub::VERSION) - if gem_version && gem_version < MIN_VERSION - raise "#{REQUIRED_LIBRARY} version #{gem_version} is not supported. Please use version >= #{MIN_VERSION}" - end - rescue => e - Chore.logger.error "Could not verify #{REQUIRED_LIBRARY} version: #{e.message}" if defined?(Chore.logger) + gem_version = Gem::Version.new(Google::Cloud::PubSub::VERSION) + if gem_version < MIN_VERSION + Chore.logger.error "#{REQUIRED_LIBRARY} version #{gem_version} is not supported. Please use version >= #{MIN_VERSION}" if defined?(Chore.logger) exit end - + Google::Cloud::PubSub.new end # Helper method to create topics and subscriptions based on the currently known list as provided by your configured Chore::Jobs diff --git a/lib/chore/queues/sqs.rb b/lib/chore/queues/sqs.rb index 1694fb9..836566e 100644 --- a/lib/chore/queues/sqs.rb +++ b/lib/chore/queues/sqs.rb @@ -8,13 +8,9 @@ def self.sqs_client require REQUIRED_LIBRARY # Verify compatible version - begin - gem_version = Gem::Version.new(Aws::SQS::GEM_VERSION) - if gem_version < MIN_VERSION - raise "#{REQUIRED_LIBRARY} version #{gem_version} is not supported. Please use version >= #{MIN_VERSION}" - end - rescue => e - Chore.logger.error "Could not verify #{REQUIRED_LIBRARY} version: #{e.message}" if defined?(Chore.logger) + gem_version = Gem::Version.new(Aws::SQS::GEM_VERSION) + if gem_version < MIN_VERSION + Chore.logger.error "#{REQUIRED_LIBRARY} version #{gem_version} is not supported. Please use version >= #{MIN_VERSION}" if defined?(Chore.logger) exit end From f4efdf540f76c491e4080e274af6e270be2508ed Mon Sep 17 00:00:00 2001 From: James Hong Date: Wed, 10 Sep 2025 10:59:58 -0700 Subject: [PATCH 26/26] [NPE-719] Broaden/Refactor Payloadhandling --- lib/chore.rb | 3 +- lib/chore/job.rb | 31 +++++-------------- lib/chore/payload_handler.rb | 24 ++++++++++++++ lib/chore/worker.rb | 2 +- spec/chore/job_spec.rb | 6 ++-- .../worker/forked_worker_strategy_spec.rb | 2 +- .../worker/helpers/work_distributor_spec.rb | 4 +-- .../worker/single_worker_strategy_spec.rb | 2 +- spec/chore/worker_spec.rb | 16 +++++----- 9 files changed, 49 insertions(+), 41 deletions(-) create mode 100644 lib/chore/payload_handler.rb diff --git a/lib/chore.rb b/lib/chore.rb index 8d6a232..3290020 100644 --- a/lib/chore.rb +++ b/lib/chore.rb @@ -14,6 +14,7 @@ require 'chore/util' require 'chore/worker' require 'chore/publisher' +require 'chore/payload_handler' # We have a number of things that can live here. I don't want to track ['queues/**','strategies/**'].each do |p| @@ -42,7 +43,7 @@ module Chore #:nodoc: :max_attempts => 1.0 / 0.0, # Infinity :dupe_on_cache_failure => false, :queue_polling_size => 10, - :payload_handler => Chore::Job, + :payload_handler => Chore::PayloadHandler, :master_procline => "chore-master-#{Chore::VERSION}", :worker_procline => "chore-worker-#{Chore::VERSION}", :consumer_sleep_interval => 1 diff --git a/lib/chore/job.rb b/lib/chore/job.rb index 53f6dc7..12a6a80 100644 --- a/lib/chore/job.rb +++ b/lib/chore/job.rb @@ -7,7 +7,6 @@ module Chore # Chore::Job is the module which gives your job classes the methods they need to be published # and run within Chore. You cannot have a Job in Chore that does not include this module module Job - extend Util # An exception to represent a job choosing to forcibly reject a given instance of itself. # The reasoning behind rejecting the job and the message that spawned it are left to @@ -28,18 +27,6 @@ def self.included(base) #:nodoc: base.extend(Hooks) end - def self.payload_class(message) - constantize(message['class']) - end - - def self.decode(data) - Encoder::JsonEncoder.decode(data) - end - - def self.payload(message) - message['args'] - end - module ClassMethods DEFAULT_OPTIONS = { } @@ -99,11 +86,6 @@ def perform_async(*args) job.perform_async(*args) end - # Resque/Sidekiq compatible serialization. No reason to change what works - def job_hash(job_params) - {:class => self.to_s, :args => job_params} - end - # The name of the configured queue, combined with an optional prefix # # @return [String] @@ -142,15 +124,16 @@ def perform(*args) # Use the current configured publisher to send this job into a queue. def perform_async(*args) - self.class.run_hooks_for(:before_publish,*args) - @chore_publisher ||= self.class.options[:publisher] + klass = self.class + klass.run_hooks_for(:before_publish,*args) + @chore_publisher ||= klass.options[:publisher] - publish_job_hash = self.class.job_hash(args) - Chore.run_hooks_for(:around_publish, self.class.prefixed_queue_name, publish_job_hash) do - @chore_publisher.publish(self.class.prefixed_queue_name,publish_job_hash) + publish_job_hash = Chore.config.payload_handler.job_hash(klass.to_s, args) + Chore.run_hooks_for(:around_publish, klass.prefixed_queue_name, publish_job_hash) do + @chore_publisher.publish(klass.prefixed_queue_name,publish_job_hash) end - self.class.run_hooks_for(:after_publish,*args) + klass.run_hooks_for(:after_publish,*args) end end #Job diff --git a/lib/chore/payload_handler.rb b/lib/chore/payload_handler.rb new file mode 100644 index 0000000..838c3bb --- /dev/null +++ b/lib/chore/payload_handler.rb @@ -0,0 +1,24 @@ +module Chore + class PayloadHandler + extend Util + + def self.payload_class(message) + constantize(message["class"]) + end + + # Takes UnitOfWork and return decoded message + def self.decode(item) + Encoder::JsonEncoder.decode(item.message) + end + + def self.payload(message) + message["args"] + end + + # Resque/Sidekiq compatible serialization. No reason to change what works + def self.job_hash(klass, job_params) + # JSON only recognizes string keys, so use strings as keys in our hash for consistency in encoding/decoding + {"class" => klass, "args" => job_params} + end + end +end diff --git a/lib/chore/worker.rb b/lib/chore/worker.rb index d9aef66..1a74270 100644 --- a/lib/chore/worker.rb +++ b/lib/chore/worker.rb @@ -77,7 +77,7 @@ def start @work.each do |item| return if @stopping begin - item.decoded_message = options[:payload_handler].decode(item.message) + item.decoded_message = options[:payload_handler].decode(item) item.klass = options[:payload_handler].payload_class(item.decoded_message) next if duplicate_work?(item) diff --git a/spec/chore/job_spec.rb b/spec/chore/job_spec.rb index ff54bf0..433c949 100644 --- a/spec/chore/job_spec.rb +++ b/spec/chore/job_spec.rb @@ -66,15 +66,15 @@ it 'should call an instance of the queue_options publisher' do args = [1,2,{:h => 'ash'}] TestJob.queue_options(:publisher => Chore::Publisher) - expect_any_instance_of(Chore::Publisher).to receive(:publish).with('test_queue',{:class => 'TestJob',:args => args}).and_return(true) + expect_any_instance_of(Chore::Publisher).to receive(:publish).with('test_queue',{"class" => 'TestJob',"args" => args}).and_return(true) TestJob.perform_async(*args) end it 'calls the around_publish hook with the correct parameters' do args = [1,2,{:h => 'ash'}] - expect(Chore).to receive(:run_hooks_for).with(:around_publish, 'test_queue', {:class => 'TestJob',:args => args}).and_call_original + expect(Chore).to receive(:run_hooks_for).with(:around_publish, 'test_queue', {"class" => 'TestJob',"args" => args}).and_call_original TestJob.queue_options(:publisher => Chore::Publisher) - expect_any_instance_of(Chore::Publisher).to receive(:publish).with('test_queue',{:class => 'TestJob',:args => args}).and_return(true) + expect_any_instance_of(Chore::Publisher).to receive(:publish).with('test_queue',{"class" => 'TestJob',"args" => args}).and_return(true) TestJob.perform_async(*args) end end diff --git a/spec/chore/strategies/worker/forked_worker_strategy_spec.rb b/spec/chore/strategies/worker/forked_worker_strategy_spec.rb index 8266f31..f2a4e4e 100644 --- a/spec/chore/strategies/worker/forked_worker_strategy_spec.rb +++ b/spec/chore/strategies/worker/forked_worker_strategy_spec.rb @@ -16,7 +16,7 @@ nil, 'test', job_timeout, - Chore::Encoder::JsonEncoder.encode(TestJob.job_hash([1,2,"3"])), + Chore::Encoder::JsonEncoder.encode(Chore::PayloadHandler.job_hash(TestJob, [1,2,"3"])), 0, consumer ) diff --git a/spec/chore/strategies/worker/helpers/work_distributor_spec.rb b/spec/chore/strategies/worker/helpers/work_distributor_spec.rb index fd7fae8..cbd475f 100644 --- a/spec/chore/strategies/worker/helpers/work_distributor_spec.rb +++ b/spec/chore/strategies/worker/helpers/work_distributor_spec.rb @@ -11,7 +11,7 @@ SecureRandom.uuid, 'test', 60, - Chore::Encoder::JsonEncoder.encode(TestJob.job_hash([1,2,"3"])), + Chore::Encoder::JsonEncoder.encode(Chore::PayloadHandler.job_hash(TestJob, [1,2,"3"])), 0 ) end @@ -89,7 +89,7 @@ SecureRandom.uuid, 'test', 60, - Chore::Encoder::JsonEncoder.encode(TestJob.job_hash([1,2,"3"])), + Chore::Encoder::JsonEncoder.encode(Chore::PayloadHandler.job_hash(TestJob, [1,2,"3"])), 0 ) worker2 = Chore::Strategy::WorkerInfo.new(2) diff --git a/spec/chore/strategies/worker/single_worker_strategy_spec.rb b/spec/chore/strategies/worker/single_worker_strategy_spec.rb index 33f7431..bbc0bd2 100644 --- a/spec/chore/strategies/worker/single_worker_strategy_spec.rb +++ b/spec/chore/strategies/worker/single_worker_strategy_spec.rb @@ -3,7 +3,7 @@ describe Chore::Strategy::SingleWorkerStrategy do let(:manager) { double('Manager') } let(:job_timeout) { 60 } - let(:job) { Chore::UnitOfWork.new(SecureRandom.uuid, nil, 'test', job_timeout, Chore::Encoder::JsonEncoder.encode(TestJob.job_hash([1,2,"3"])), 0) } + let(:job) { Chore::UnitOfWork.new(SecureRandom.uuid, nil, 'test', job_timeout, Chore::Encoder::JsonEncoder.encode(Chore::PayloadHandler.job_hash(TestJob, [1,2,"3"])), 0) } subject { described_class.new(manager) } describe '#stop!' do diff --git a/spec/chore/worker_spec.rb b/spec/chore/worker_spec.rb index 586121a..9b3e22d 100644 --- a/spec/chore/worker_spec.rb +++ b/spec/chore/worker_spec.rb @@ -43,11 +43,11 @@ def perform(first, second) let(:consumer) { double('consumer', :complete => nil, :reject => nil) } let(:job_args) { [1,2,'3'] } - let(:job) { SimpleJob.job_hash(job_args) } + let(:job) { Chore::PayloadHandler.job_hash(SimpleJob, job_args) } it 'should use a default payload handler' do worker = Chore::Worker.new - expect(worker.options[:payload_handler]).to eq(Chore::Job) + expect(worker.options[:payload_handler]).to eq(Chore::PayloadHandler) end shared_examples_for "a worker" do @@ -82,11 +82,11 @@ def perform(first, second) context 'when the value being deduped on is unique' do let(:job_args) { [rand,2,'3'] } let(:encoded_job) { Chore::Encoder::JsonEncoder.encode(job) } - let(:job) { SimpleDedupeJob.job_hash(job_args) } + let(:job) { Chore::PayloadHandler.job_hash(SimpleDedupeJob, job_args) } it 'should call complete for each unique value' do allow(consumer).to receive(:duplicate_message?).and_return(false) work = [] - work << Chore::UnitOfWork.new(1, nil, 'dedupe_test', 60, Chore::Encoder::JsonEncoder.encode(SimpleDedupeJob.job_hash([rand,2,'3'])), 0, consumer) + work << Chore::UnitOfWork.new(1, nil, 'dedupe_test', 60, Chore::Encoder::JsonEncoder.encode(Chore::PayloadHandler.job_hash(SimpleDedupeJob, [rand,2,'3'])), 0, consumer) expect(SimpleDedupeJob).to receive(:perform).exactly(1).times expect(consumer).to receive(:complete).exactly(1).times Chore::Worker.start(work, {:payload_handler => payload_handler}) @@ -96,9 +96,9 @@ def perform(first, second) context 'when the dedupe lambda does not take the same number of arguments as perform' do it 'should raise an error and not complete the job' do work = [] - work << Chore::UnitOfWork.new(1, nil, 'invalid_dedupe_test', 60, Chore::Encoder::JsonEncoder.encode(InvalidDedupeJob.job_hash([rand,2,'3'])), 0, consumer) - work << Chore::UnitOfWork.new(2, nil, 'invalid_dedupe_test', 60, Chore::Encoder::JsonEncoder.encode(InvalidDedupeJob.job_hash([rand,2,'3'])), 0, consumer) - work << Chore::UnitOfWork.new(1, nil, 'invalid_dedupe_test', 60, Chore::Encoder::JsonEncoder.encode(InvalidDedupeJob.job_hash([rand,2,'3'])), 0, consumer) + work << Chore::UnitOfWork.new(1, nil, 'invalid_dedupe_test', 60, Chore::Encoder::JsonEncoder.encode(Chore::PayloadHandler.job_hash(InvalidDedupeJob, [rand,2,'3'])), 0, consumer) + work << Chore::UnitOfWork.new(2, nil, 'invalid_dedupe_test', 60, Chore::Encoder::JsonEncoder.encode(Chore::PayloadHandler.job_hash(InvalidDedupeJob, [rand,2,'3'])), 0, consumer) + work << Chore::UnitOfWork.new(1, nil, 'invalid_dedupe_test', 60, Chore::Encoder::JsonEncoder.encode(Chore::PayloadHandler.job_hash(InvalidDedupeJob, [rand,2,'3'])), 0, consumer) expect(consumer).not_to receive(:complete) Chore::Worker.start(work, {:payload_handler => payload_handler}) end @@ -108,7 +108,7 @@ def perform(first, second) describe "with default payload handler" do let(:encoded_job) { Chore::Encoder::JsonEncoder.encode(job) } - let(:payload_handler) { Chore::Job } + let(:payload_handler) { Chore::PayloadHandler } let(:payload) {job_args} it_behaves_like "a worker" end