Merge pull request #133 from KnapsackPro/connect_to_queue

Add support for an attempt to connect to existing Queue on API side to reduce slow requests number

CHANGELOG.md

@@ -1,5 +1,13 @@
 # Change Log
 
+### 2.10.0
+
+* Add support for an attempt to connect to existing Queue on API side to reduce slow requests number
+
+    https://github.com/KnapsackPro/knapsack_pro-ruby/pull/133
+
+https://github.com/KnapsackPro/knapsack_pro-ruby/compare/v2.9.0...v2.10.0
+
 ### 2.9.0
 
 * Use `Process.clock_gettime` to measure track execution time

lib/knapsack_pro/base_allocator_builder.rb

@@ -28,6 +28,10 @@ def fallback_mode_test_files
       all_test_files_to_run
     end
 
+    def lazy_fallback_mode_test_files
+      lambda { fallback_mode_test_files }
+    end
+
     # detect test files present on the disk that should be run
     # this may include some fast test files + slow test files split by test cases
     def fast_and_slow_test_files_to_run
@@ -63,6 +67,10 @@ def fast_and_slow_test_files_to_run
       end
     end
 
+    def lazy_fast_and_slow_test_files_to_run
+      lambda { fast_and_slow_test_files_to_run }
+    end
+
     private
 
     attr_reader :adapter_class

lib/knapsack_pro/client/api/v1/queues.rb

@@ -3,19 +3,22 @@ module Client
     module API
       module V1
         class Queues < Base
+          CODE_ATTEMPT_CONNECT_TO_QUEUE_FAILED = 'ATTEMPT_CONNECT_TO_QUEUE_FAILED'
+
           class << self
             def queue(args)
               request_hash = {
                 :fixed_queue_split => KnapsackPro::Config::Env.fixed_queue_split,
                 :can_initialize_queue => args.fetch(:can_initialize_queue),
+                :attempt_connect_to_queue => args.fetch(:attempt_connect_to_queue),
                 :commit_hash => args.fetch(:commit_hash),
                 :branch => args.fetch(:branch),
                 :node_total => args.fetch(:node_total),
                 :node_index => args.fetch(:node_index),
                 :node_build_id => KnapsackPro::Config::Env.ci_node_build_id,
               }
 
-              if request_hash[:can_initialize_queue]
+              if request_hash[:can_initialize_queue] && !request_hash[:attempt_connect_to_queue]
                 request_hash.merge!({
                   :test_files => args.fetch(:test_files)
                 })

lib/knapsack_pro/client/connection.rb

@@ -25,6 +25,11 @@ def errors?
         !!(response_body && (response_body['errors'] || response_body['error']))
       end
 
+      def api_code
+        return unless response_body
+        response_body['code']
+      end
+
       def server_error?
         status = http_response.code.to_i
         status >= 500 && status < 600

lib/knapsack_pro/queue_allocator.rb

@@ -1,8 +1,8 @@
 module KnapsackPro
   class QueueAllocator
     def initialize(args)
-      @fast_and_slow_test_files_to_run = args.fetch(:fast_and_slow_test_files_to_run)
-      @fallback_mode_test_files = args.fetch(:fallback_mode_test_files)
+      @lazy_fast_and_slow_test_files_to_run = args.fetch(:lazy_fast_and_slow_test_files_to_run)
+      @lazy_fallback_mode_test_files = args.fetch(:lazy_fallback_mode_test_files)
       @ci_node_total = args.fetch(:ci_node_total)
       @ci_node_index = args.fetch(:ci_node_index)
       @ci_node_build_id = args.fetch(:ci_node_build_id)
@@ -11,9 +11,18 @@ def initialize(args)
 
     def test_file_paths(can_initialize_queue, executed_test_files)
       return [] if @fallback_activated
-      action = build_action(can_initialize_queue)
+      action = build_action(can_initialize_queue, attempt_connect_to_queue: can_initialize_queue)
       connection = KnapsackPro::Client::Connection.new(action)
       response = connection.call
+
+      # when attempt to connect to existing queue on API side failed because queue does not exist yet
+      if can_initialize_queue && connection.success? && connection.api_code == KnapsackPro::Client::API::V1::Queues::CODE_ATTEMPT_CONNECT_TO_QUEUE_FAILED
+        # make attempt to initalize a new queue on API side
+        action = build_action(can_initialize_queue, attempt_connect_to_queue: false)
+        connection = KnapsackPro::Client::Connection.new(action)
+        response = connection.call
+      end
+
       if connection.success?
         raise ArgumentError.new(response) if connection.errors?
         prepare_test_files(response)
@@ -37,40 +46,58 @@ def test_file_paths(can_initialize_queue, executed_test_files)
 
     private
 
-    attr_reader :fast_and_slow_test_files_to_run,
-      :fallback_mode_test_files,
+    attr_reader :lazy_fast_and_slow_test_files_to_run,
+      :lazy_fallback_mode_test_files,
       :ci_node_total,
       :ci_node_index,
       :ci_node_build_id,
       :repository_adapter
 
+    # This method might be slow because it reads test files from disk.
+    # This method can be very slow (a few seconds or more) when you use RSpec split by test examples feature because RSpec needs to generate JSON report with test examples ids
+    def lazy_loaded_fast_and_slow_test_files_to_run
+      @lazy_loaded_fast_and_slow_test_files_to_run ||= lazy_fast_and_slow_test_files_to_run.call
+    end
+
     def encrypted_test_files
-      KnapsackPro::Crypto::Encryptor.call(fast_and_slow_test_files_to_run)
+      KnapsackPro::Crypto::Encryptor.call(lazy_loaded_fast_and_slow_test_files_to_run)
     end
 
     def encrypted_branch
       KnapsackPro::Crypto::BranchEncryptor.call(repository_adapter.branch)
     end
 
-    def build_action(can_initialize_queue)
+    def build_action(can_initialize_queue, attempt_connect_to_queue:)
+      # read test files from disk only when needed because it can be slow operation
+      test_files =
+        if can_initialize_queue && !attempt_connect_to_queue
+          encrypted_test_files
+        end
+
       KnapsackPro::Client::API::V1::Queues.queue(
         can_initialize_queue: can_initialize_queue,
+        attempt_connect_to_queue: attempt_connect_to_queue,
         commit_hash: repository_adapter.commit_hash,
         branch: encrypted_branch,
         node_total: ci_node_total,
         node_index: ci_node_index,
         node_build_id: ci_node_build_id,
-        test_files: encrypted_test_files,
+        test_files: test_files,
       )
     end
 
     def prepare_test_files(response)
-      decrypted_test_files = KnapsackPro::Crypto::Decryptor.call(fast_and_slow_test_files_to_run, response['test_files'])
-      KnapsackPro::TestFilePresenter.paths(decrypted_test_files)
+      # when encryption is disabled we can avoid calling slow method lazy_loaded_fast_and_slow_test_files_to_run
+      if KnapsackPro::Config::Env.test_files_encrypted?
+        decrypted_test_files = KnapsackPro::Crypto::Decryptor.call(lazy_loaded_fast_and_slow_test_files_to_run, response['test_files'])
+        KnapsackPro::TestFilePresenter.paths(decrypted_test_files)
+      else
+        KnapsackPro::TestFilePresenter.paths(response['test_files'])
+      end
     end
 
     def fallback_test_files(executed_test_files)
-      test_flat_distributor = KnapsackPro::TestFlatDistributor.new(fallback_mode_test_files, ci_node_total)
+      test_flat_distributor = KnapsackPro::TestFlatDistributor.new(lazy_fallback_mode_test_files.call, ci_node_total)
       test_files_for_node_index = test_flat_distributor.test_files_for_node(ci_node_index)
       KnapsackPro::TestFilePresenter.paths(test_files_for_node_index) - executed_test_files
     end

lib/knapsack_pro/queue_allocator_builder.rb

@@ -2,8 +2,8 @@ module KnapsackPro
   class QueueAllocatorBuilder < BaseAllocatorBuilder
     def allocator
       KnapsackPro::QueueAllocator.new(
-        fast_and_slow_test_files_to_run: fast_and_slow_test_files_to_run,
-        fallback_mode_test_files: fallback_mode_test_files,
+        lazy_fast_and_slow_test_files_to_run: lazy_fast_and_slow_test_files_to_run,
+        lazy_fallback_mode_test_files: lazy_fallback_mode_test_files,
         ci_node_total: env.ci_node_total,
         ci_node_index: env.ci_node_index,
         ci_node_build_id: env.ci_node_build_id,

spec/knapsack_pro/client/api/v1/queues_spec.rb

@@ -11,6 +11,7 @@
     subject do
       described_class.queue(
         can_initialize_queue: can_initialize_queue,
+        attempt_connect_to_queue: attempt_connect_to_queue,
         commit_hash: commit_hash,
         branch: branch,
         node_total: node_total,
@@ -24,8 +25,33 @@
       expect(KnapsackPro::Config::Env).to receive(:ci_node_build_id).and_return(node_build_id)
     end
 
-    context 'when can_initialize_queue=true' do
+    context 'when can_initialize_queue=true and attempt_connect_to_queue=true' do
       let(:can_initialize_queue) { true }
+      let(:attempt_connect_to_queue) { true }
+
+      it 'does not send test_files among other params' do
+        action = double
+        expect(KnapsackPro::Client::API::Action).to receive(:new).with({
+          endpoint_path: '/v1/queues/queue',
+          http_method: :post,
+          request_hash: {
+            fixed_queue_split: fixed_queue_split,
+            can_initialize_queue: can_initialize_queue,
+            attempt_connect_to_queue: attempt_connect_to_queue,
+            commit_hash: commit_hash,
+            branch: branch,
+            node_total: node_total,
+            node_index: node_index,
+            node_build_id: node_build_id,
+          }
+        }).and_return(action)
+        expect(subject).to eq action
+      end
+    end
+
+    context 'when can_initialize_queue=true and attempt_connect_to_queue=false' do
+      let(:can_initialize_queue) { true }
+      let(:attempt_connect_to_queue) { false }
 
       it 'sends test_files among other params' do
         action = double
@@ -35,6 +61,7 @@
           request_hash: {
             fixed_queue_split: fixed_queue_split,
             can_initialize_queue: can_initialize_queue,
+            attempt_connect_to_queue: attempt_connect_to_queue,
             commit_hash: commit_hash,
             branch: branch,
             node_total: node_total,
@@ -47,8 +74,9 @@
       end
     end
 
-    context 'when can_initialize_queue=false' do
+    context 'when can_initialize_queue=false and attempt_connect_to_queue=false' do
       let(:can_initialize_queue) { false }
+      let(:attempt_connect_to_queue) { false }
 
       it 'does not send test_files among other params' do
         action = double
@@ -58,6 +86,7 @@
           request_hash: {
             fixed_queue_split: fixed_queue_split,
             can_initialize_queue: can_initialize_queue,
+            attempt_connect_to_queue: attempt_connect_to_queue,
             commit_hash: commit_hash,
             branch: branch,
             node_total: node_total,

spec/knapsack_pro/queue_allocator_builder_spec.rb

@@ -8,11 +8,11 @@
     subject { allocator_builder.allocator }
 
     before do
-      fast_and_slow_test_files_to_run = double
-      expect(allocator_builder).to receive(:fast_and_slow_test_files_to_run).and_return(fast_and_slow_test_files_to_run)
+      lazy_fast_and_slow_test_files_to_run = double
+      expect(allocator_builder).to receive(:lazy_fast_and_slow_test_files_to_run).and_return(lazy_fast_and_slow_test_files_to_run)
 
-      fallback_mode_test_files = double
-      expect(allocator_builder).to receive(:fallback_mode_test_files).and_return(fallback_mode_test_files)
+      lazy_fallback_mode_test_files = double
+      expect(allocator_builder).to receive(:lazy_fallback_mode_test_files).and_return(lazy_fallback_mode_test_files)
 
       repository_adapter = double
       expect(KnapsackPro::RepositoryAdapterInitiator).to receive(:call).and_return(repository_adapter)
@@ -25,8 +25,8 @@
       expect(KnapsackPro::Config::Env).to receive(:ci_node_build_id).and_return(ci_node_build_id)
 
       expect(KnapsackPro::QueueAllocator).to receive(:new).with(
-        fast_and_slow_test_files_to_run: fast_and_slow_test_files_to_run,
-        fallback_mode_test_files: fallback_mode_test_files,
+        lazy_fast_and_slow_test_files_to_run: lazy_fast_and_slow_test_files_to_run,
+        lazy_fallback_mode_test_files: lazy_fallback_mode_test_files,
         ci_node_total: ci_node_total,
         ci_node_index: ci_node_index,
         ci_node_build_id: ci_node_build_id,