Implement PR feedback

Author: hschne

lib/litestream/commands.rb

@@ -94,9 +94,12 @@ def executable(exe_path: DEFAULT_DIR)
         exe_file
       end
 
+      # Replicate can be run either as a fork or in the same process, depending on the context.
+      # Puma will start replication as a forked process, while running replication from a rake
+      # tasks won't.
       def replicate(async: false, **argv)
         cmd = prepare("replicate", argv)
-        run_async(cmd, async: async)
+        run_replicate(cmd, async: async)
       rescue
         raise CommandFailedException, "Failed to execute `#{cmd.join(" ")}`"
       end
@@ -164,31 +167,16 @@ def run(cmd, tabled_output:)
         rows.map { keys.zip(_1).to_h }
       end
 
-      def run_async(cmd, async:)
+      def run_replicate(cmd, async:)
         if async
           exec(*cmd) if fork.nil?
         else
+          # When running in-process, we capture output continuously and write to stdout.
           IO.popen(cmd, err: [:child, :out]) do |io|
             io.each_line { |line| puts line }
           end
         end
       end
-
-      module Output
-        class << self
-          def format(data)
-            headers = data.first.keys.map(&:to_s)
-            widths = headers.map.with_index { |h, i|
-              [h.length, data.map { |r| r[data.first.keys[i]].to_s.length }.max].max
-            }
-
-            format_str = widths.map { |w| "%-#{w}s" }.join("  ")
-            ([headers] + data.map(&:values)).map { |row|
-              sprintf(format_str, *row.map(&:to_s))
-            }.join("\n")
-          end
-        end
-      end
     end
   end
 end

lib/tasks/litestream_tasks.rake

@@ -63,8 +63,8 @@ namespace :litestream do
     if (separator_index = ARGV.index("--"))
       ARGV.slice(separator_index + 1, ARGV.length)
         .map { |pair| pair.split("=") }
-        .each { |opt| options[opt[0].to_sym] = opt[1] || nil }
+        .each { |opt| options[opt[0]] = opt[1] || nil }
     end
-    options.symbolize_keys
+    options.symbolize_keys!
   end
 end

test/litestream/test_commands.rb

@@ -27,7 +27,7 @@ def test_replicate_with_no_options
         assert_equal "--config", argv[0]
         assert_match Regexp.new("dummy/config/litestream.yml"), argv[1]
       end
-      Litestream::Commands.stub :run_async, stub do
+      Litestream::Commands.stub :run_replicate, stub do
         Litestream::Commands.replicate
       end
     end
@@ -42,7 +42,7 @@ def test_replicate_with_boolean_option
         assert_match Regexp.new("dummy/config/litestream.yml"), argv[1]
         assert_equal "--no-expand-env", argv[2]
       end
-      Litestream::Commands.stub :run_async, stub do
+      Litestream::Commands.stub :run_replicate, stub do
         Litestream::Commands.replicate("--no-expand-env" => nil)
       end
     end
@@ -58,7 +58,7 @@ def test_replicate_with_string_option
         assert_equal "--exec", argv[2]
         assert_equal "command", argv[3]
       end
-      Litestream::Commands.stub :run_async, stub do
+      Litestream::Commands.stub :run_replicate, stub do
         Litestream::Commands.replicate("--exec" => "command")
       end
     end
@@ -74,7 +74,7 @@ def test_replicate_with_symbol_option
         assert_equal "--exec", argv[2]
         assert_equal "command", argv[3]
       end
-      Litestream::Commands.stub :run_async, stub do
+      Litestream::Commands.stub :run_replicate, stub do
         Litestream::Commands.replicate("--exec": "command")
       end
     end
@@ -88,15 +88,15 @@ def test_replicate_with_config_option
         assert_equal "--config", argv[0]
         assert_equal "CONFIG", argv[1]
       end
-      Litestream::Commands.stub :run_async, stub do
+      Litestream::Commands.stub :run_replicate, stub do
         Litestream::Commands.replicate("--config" => "CONFIG")
       end
     end
 
     def test_replicate_sets_replica_bucket_env_var_from_config_when_env_var_not_set
       Litestream.replica_bucket = "mybkt"
 
-      Litestream::Commands.stub :run_async, nil do
+      Litestream::Commands.stub :run_replicate, nil do
         Litestream::Commands.replicate
       end
 
@@ -108,7 +108,7 @@ def test_replicate_sets_replica_bucket_env_var_from_config_when_env_var_not_set
     def test_replicate_sets_replica_key_id_env_var_from_config_when_env_var_not_set
       Litestream.replica_key_id = "mykey"
 
-      Litestream::Commands.stub :run_async, nil do
+      Litestream::Commands.stub :run_replicate, nil do
         Litestream::Commands.replicate
       end
 
@@ -120,7 +120,7 @@ def test_replicate_sets_replica_key_id_env_var_from_config_when_env_var_not_set
     def test_replicate_sets_replica_access_key_env_var_from_config_when_env_var_not_set
       Litestream.replica_access_key = "access"
 
-      Litestream::Commands.stub :run_async, nil do
+      Litestream::Commands.stub :run_replicate, nil do
         Litestream::Commands.replicate
       end
 
@@ -134,7 +134,7 @@ def test_replicate_sets_all_env_vars_from_config_when_env_vars_not_set
       Litestream.replica_key_id = "mykey"
       Litestream.replica_access_key = "access"
 
-      Litestream::Commands.stub :run_async, nil do
+      Litestream::Commands.stub :run_replicate, nil do
         Litestream::Commands.replicate
       end
 
@@ -152,7 +152,7 @@ def test_replicate_does_not_set_env_var_from_config_when_env_vars_already_set
       Litestream.replica_key_id = "mykey"
       Litestream.replica_access_key = "access"
 
-      Litestream::Commands.stub :run_async, nil do
+      Litestream::Commands.stub :run_replicate, nil do
         Litestream::Commands.replicate
       end