Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document that DefaultRackup et al. was removed [ci skip] #2990

Merged
merged 2 commits into from
Oct 14, 2022

Conversation

dentarg
Copy link
Member

@dentarg dentarg commented Oct 14, 2022

Close #2989

@dentarg dentarg merged commit cfb0477 into puma:master Oct 14, 2022
@dentarg dentarg deleted the 6.0_docs_fixes branch October 14, 2022 18:33
@bf4
Copy link
Contributor

bf4 commented Oct 14, 2022

FWIW, I've now added a test that puma boots to our test suite. Sharing for anyone who wants to give it a shot

  it "boots puma" do
    args = ["bin/task-puma_boot"]
    process_result = system(*args)
    expect(process_result).to eq(true)
  end

and which calls a script bin/task-puma_boot which is also called by our release/deploy script

#!/usr/bin/env ruby

# https://github.com/puma/puma/pull/2990#issuecomment-1279557257
# https://github.com/puma/puma/blob/v6.0.0/test/test_rack_handler.rb
class TaskPumaBoot

  def self.test_it_boots_puma(dry_run: false)
    instance = new
    instance.dry_run = dry_run
    instance.test_it_boots_puma
  end

  attr_accessor :response
  attr_accessor :launcher
  attr_accessor :dry_run

  def test_it_boots_puma
    require_dependencies

    self.response = nil
    app = proc { |env|
      [200, {}, ["got #{env["PATH_INFO"]}"]]
    }
    host = "127.0.0.1"
    port = get_unique_port
    opts = {Host: host, Port: port}
    in_handler(app, opts) do |_launcher|
      self.response = hit(["http://#{host}:#{port}/test"])
      actual = response.first
      expected = "got /test"
      if actual != expected
        message  ="Expected #{expected.inspect}; Got #{actual.inspect}"
        if dry_run
          $stderr.puts message
        else
          abort message
        end
      end
    end
    if dry_run
      :success
    else
      exit 0 # Success!
    end
  end

  def require_dependencies
    require "timeout"
    require "net/http"
    require "pathname"
    require "fileutils"
    FileUtils.mkdir_p(app_root.join("tmp/pids"))
    require "rack/handler/puma"
  end

  def in_handler(app, options = {})
    options[:Port] ||= 0
    options[:Silent] = true

    instance = self
    instance.launcher = nil
    thread = Thread.new do
      Rack::Handler::Puma.run(app, **options) do |s, p|
        instance.launcher = s
      end
    end

    # Wait for launcher to boot
    Timeout.timeout(10) do
      sleep 0.5 until instance.launcher
    end
    sleep 1.5 unless Puma::IS_MRI

    yield instance.launcher
  ensure
    instance.launcher&.stop
    thread&.join
  end

  # Either takes a string to do a get request against, or a tuple of [URI, HTTP] where
  # HTTP is some kind of Net::HTTP request object (POST, HEAD, etc.)
  def hit(uris)
    uris.map do |u|
      response =
        if u.kind_of? String
          Net::HTTP.get(URI.parse(u))
        else
          url = URI.parse(u[0])
          Net::HTTP.new(url.host, url.port).start { |h| h.request(u[1]) }
        end

      if response.nil? || response.to_s.strip.empty?
        message = "Didn't get a response: #{u}"
        if dry_run
          $stderr.puts message
        else
          abort message
        end
      end
      response
    end
  end

  def get_unique_port
    TCPServer.open("127.0.0.1", 0) do |server|
      server.connect_address.ip_port
    end
  end

  def app_root
    @app_root ||= Pathname(File.expand_path("../..", __FILE__))
  end
end
if $0 == __FILE__
  TaskPumaBoot.test_it_boots_puma
end

@dentarg
Copy link
Member Author

dentarg commented Oct 15, 2022

@bf4 I have tests like that too, but they're a bit different from yours, I parse the Procfile and start that command, see https://github.com/Starkast/wikimum/blob/95777e39844847afc0cbb68acb3780c07e339d5b/test/integration/app_boot_test.rb

@bf4
Copy link
Contributor

bf4 commented Oct 16, 2022

yeah, https://github.com/Starkast/wikimum/blob/95777e39844847afc0cbb68acb3780c07e339d5b/test/integration/app_boot_test.rb looks pretty similar in the important bits, though it tests the worker, not web process. I actually think mine (which is extracted from Puma) could probably be simpler, but I threw this spaghetti at the wall and stuck, so I moved on.

Someone else made this shell script https://gist.github.com/vojtad/3649aa51c4dd9f9c5ae148e3b3afbbbe

@MSP-Greg MSP-Greg added the docs label Dec 18, 2022
@bf4
Copy link
Contributor

bf4 commented Feb 13, 2023

Seems my test task above broke in 6.1.0 since #3061

diff --git a/bin/task-puma_boot b/bin/task-puma_boot
index c5a9f8a1c5..86e44b48fc 100755
--- a/bin/task-puma_boot
+++ b/bin/task-puma_boot
@@ -1,6 +1,7 @@
 #!/usr/bin/env ruby
 
 # https://github.com/puma/puma/pull/2990#issuecomment-1279557257
+# https://github.com/puma/puma/pull/3061
 # https://github.com/puma/puma/blob/v6.0.0/test/test_rack_handler.rb
 class TaskPumaBoot
 
@@ -50,6 +51,12 @@ class TaskPumaBoot
     require "pathname"
     require "fileutils"
     FileUtils.mkdir_p(app_root.join("tmp/pids"))
+    require "bundler/setup"
+    # on Rack 3 is
+    # require "rackup"
+    # @rack_handler_mod = ::Rackup::Handler
+    require "rack"
+    @rack_handler_mod = ::Rack::Handler
     require "rack/handler/puma"
   end
 
@@ -60,7 +67,7 @@ class TaskPumaBoot
     instance = self
     instance.launcher = nil
     thread = Thread.new do
-      Rack::Handler::Puma.run(app, **options) do |s, p|
+      @rack_handler_mod::Puma.run(app, **options) do |s, p|
         instance.launcher = s
       end
     end

@MSP-Greg
Copy link
Member

@bf4

I hate it when that happens. My mistake, I was focused on making sure the rackup bin file worked, forgot about starting Puma as you have. I'll have a PR soon...

@MSP-Greg
Copy link
Member

@bf4

I believe #3080 fixes the issue, so the patch you listed shouldn't be needed.

If you have time to check it, it would be appreciated.

@dentarg
Copy link
Member Author

dentarg commented Feb 13, 2023

yeah, https://github.com/Starkast/wikimum/blob/95777e39844847afc0cbb68acb3780c07e339d5b/test/integration/app_boot_test.rb looks pretty similar in the important bits, though it tests the worker, not web process

Not sure I follow... :) The worker handles web requests? The tests makes web requests

@bf4
Copy link
Contributor

bf4 commented Feb 13, 2023

@dentarg yeah, I misread the code first time around. 🙃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Uninitialized constant Puma::DSL::DefaultRackup
3 participants