From 03ecfdc3616902acf75fc455b61ac9444120929b Mon Sep 17 00:00:00 2001 From: Stephen Binns Date: Tue, 23 Jul 2024 08:35:01 +0100 Subject: [PATCH 1/7] Bump rack and webrick versions This prevents upgrades in upstream gems --- .ruby-version | 2 +- que.gemspec | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.ruby-version b/.ruby-version index 2451c27..bea438e 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -3.0.7 +3.3.1 diff --git a/que.gemspec b/que.gemspec index 1126f8d..129996a 100644 --- a/que.gemspec +++ b/que.gemspec @@ -27,8 +27,8 @@ Gem::Specification.new do |spec| # This is highly non ideal, but unless we properly fork, we have to do this for now. spec.add_dependency "prometheus-client" - spec.add_dependency "rack", "~> 2.0" - spec.add_dependency "webrick", "~> 1.7" + spec.add_dependency "rack", "~> 3.0" + spec.add_dependency "webrick", "~> 1.8" spec.add_runtime_dependency "activesupport" spec.metadata["rubygems_mfa_required"] = "true" From 381291a8b9a0e17d93791a549b48f41a3a861351 Mon Sep 17 00:00:00 2001 From: Stephen Binns Date: Tue, 6 Aug 2024 10:00:52 +0100 Subject: [PATCH 2/7] Support rack 3 and add smoke test We were also missing some dependencies so these have been added too. --- .github/workflows/tests.yml | 24 ++++++++++++++++++-- bin/que | 44 +++++++++++++++++++++++++++++-------- que.gemspec | 6 +++-- 3 files changed, 61 insertions(+), 13 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index b5b725e..445b91b 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -19,6 +19,26 @@ jobs: run: | bundle exec rubocop --extra-details --display-style-guide --no-server --parallel + smoke_test: + strategy: + fail-fast: false + matrix: + ruby_version: ["3.0", "3.1", "3.2", "3.3"] + + runs-on: ubuntu-latest + env: + BUNDLE_RUBYGEMS__PKG__GITHUB__COM: gocardless-robot-readonly:${{ secrets.GITHUB_TOKEN }} + steps: + - uses: actions/checkout@v4 + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + bundler-cache: true + ruby-version: "${{ matrix.ruby-version }}" + - name: Start bin/que + run: | + bin/que ./lib/que.rb --metrics-port=8080 --ci + rspec: strategy: fail-fast: false @@ -40,7 +60,7 @@ jobs: --health-interval 10s --health-timeout 5s --health-retries 10 - + env: PGDATABASE: que-test PGUSER: ubuntu @@ -56,4 +76,4 @@ jobs: ruby-version: "${{ matrix.ruby-version }}" - name: Run specs run: | - bundle exec rspec \ No newline at end of file + bundle exec rspec diff --git a/bin/que b/bin/que index ec9ff68..1d6cc39 100755 --- a/bin/que +++ b/bin/que @@ -7,8 +7,14 @@ require "ostruct" require "que" require "rack" require "prometheus/middleware/exporter" +require "prometheus_gcstat" require "webrick" +if Rack.release[0] == "3" + # Required if using Rack 3.x + require "rackup" +end + $stdout.sync = true options = OpenStruct.new @@ -67,6 +73,10 @@ OptionParser.new do |opts| $stdout.puts opts exit 0 end + + opts.on("--ci", "Don't wait for sigterm exit after boot") do + options.ci = true + end end.parse!(ARGV) # rubocop:enable Layout/LineLength @@ -150,15 +160,31 @@ if options.metrics_port end, ) - Rack::Handler::WEBrick.run( - app, - Host: "0.0.0.0", - Port: options.metrics_port, - Logger: WEBrick::Log.new("/dev/null"), - AccessLog: [], - ) + host = "0.0.0.0" + logger = WEBrick::Log.new("/dev/null") + + if Rack.release[0] == "3" + Rackup::Handler::WEBrick.run( + app, + Host: host, + Port: options.metrics_port, + Logger: logger, + AccessLog: [], + ) + else + Rack::Handler::WEBrick.run( + app, + Host: host, + Port: options.metrics_port, + Logger: logger, + AccessLog: [], + ) + end end end -wait_for_signals("INT", "TERM") -worker_group.stop(timeout) +# For a basic CI check we ensure the app boots etc +unless options.ci + wait_for_signals("INT", "TERM") + worker_group.stop(timeout) +end diff --git a/que.gemspec b/que.gemspec index 129996a..c784645 100644 --- a/que.gemspec +++ b/que.gemspec @@ -26,9 +26,11 @@ Gem::Specification.new do |spec| # instead, and in any other clients of `Que`. # This is highly non ideal, but unless we properly fork, we have to do this for now. spec.add_dependency "prometheus-client" + spec.add_dependency "prometheus_gcstat" - spec.add_dependency "rack", "~> 3.0" - spec.add_dependency "webrick", "~> 1.8" + spec.add_dependency "rack", ">= 2", "< 4" + spec.add_dependency "rackup" + spec.add_dependency "webrick" spec.add_runtime_dependency "activesupport" spec.metadata["rubygems_mfa_required"] = "true" From 502ecfc9919f25e16cabd204e1ac3ddafdfc6438 Mon Sep 17 00:00:00 2001 From: Stephen Binns Date: Tue, 6 Aug 2024 10:05:48 +0100 Subject: [PATCH 3/7] Try bundle exec --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 445b91b..8090816 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -37,7 +37,7 @@ jobs: ruby-version: "${{ matrix.ruby-version }}" - name: Start bin/que run: | - bin/que ./lib/que.rb --metrics-port=8080 --ci + bundle exec bin/que ./lib/que.rb --metrics-port=8080 --ci rspec: strategy: From 5dd50567b1a7d9b2aa975606d8a5037b55256edb Mon Sep 17 00:00:00 2001 From: Stephen Binns Date: Tue, 6 Aug 2024 10:10:13 +0100 Subject: [PATCH 4/7] Make CI check more realistic --- .github/workflows/tests.yml | 19 +++++++++++++++++++ bin/que | 6 ++++-- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 8090816..0006b60 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -26,7 +26,26 @@ jobs: ruby_version: ["3.0", "3.1", "3.2", "3.3"] runs-on: ubuntu-latest + services: + postgres: + image: postgres:14.2 + env: + POSTGRES_DB: que-test + POSTGRES_USER: ubuntu + POSTGRES_PASSWORD: password + ports: + - 5432:5432 + options: >- + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 10 + env: + PGDATABASE: que-test + PGUSER: ubuntu + PGPASSWORD: password + PGHOST: localhost BUNDLE_RUBYGEMS__PKG__GITHUB__COM: gocardless-robot-readonly:${{ secrets.GITHUB_TOKEN }} steps: - uses: actions/checkout@v4 diff --git a/bin/que b/bin/que index 1d6cc39..d3c4ad8 100755 --- a/bin/que +++ b/bin/que @@ -183,8 +183,10 @@ if options.metrics_port end end -# For a basic CI check we ensure the app boots etc +# For a basic CI check we just want to ensure the app boots so don't want to +# block the main thread, so this will just exit immediately. unless options.ci wait_for_signals("INT", "TERM") - worker_group.stop(timeout) end + +worker_group.stop(timeout) From 3b0cc6ef5c5f450125c416b0dd69516ec8dc3f7e Mon Sep 17 00:00:00 2001 From: Stephen Binns Date: Tue, 6 Aug 2024 10:13:30 +0100 Subject: [PATCH 5/7] Remove dependency on promgcstat as this is a private gem We can load it for CI but for the benchmarking step we push a docker image which can't be built without adding the token in. As this is a public repo I'm not sure we want to add it in. --- que.gemspec | 1 - 1 file changed, 1 deletion(-) diff --git a/que.gemspec b/que.gemspec index c784645..2c0b74c 100644 --- a/que.gemspec +++ b/que.gemspec @@ -26,7 +26,6 @@ Gem::Specification.new do |spec| # instead, and in any other clients of `Que`. # This is highly non ideal, but unless we properly fork, we have to do this for now. spec.add_dependency "prometheus-client" - spec.add_dependency "prometheus_gcstat" spec.add_dependency "rack", ">= 2", "< 4" spec.add_dependency "rackup" From 5cb524ae2fd450b32ed952f8e8ac8f138ab7f14a Mon Sep 17 00:00:00 2001 From: Stephen Binns Date: Tue, 6 Aug 2024 10:34:32 +0100 Subject: [PATCH 6/7] Fixup CI step We need to establish a connection to the database manually --- bin/que | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/bin/que b/bin/que index d3c4ad8..148824d 100755 --- a/bin/que +++ b/bin/que @@ -106,6 +106,12 @@ secondary_queues = options.secondary_queues || [] Que.logger ||= Logger.new($stdout) +if options.ci + require "active_record" + + Que.connection = ActiveRecord +end + begin Que.logger.level = Logger.const_get(log_level.upcase) if log_level rescue NameError From 7ac29cee87e6572ca7a1b7e4593bfcf98f6ef1f6 Mon Sep 17 00:00:00 2001 From: Stephen Binns Date: Tue, 6 Aug 2024 10:37:53 +0100 Subject: [PATCH 7/7] Add db migrate to smoke testing --- bin/que | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/bin/que b/bin/que index 148824d..b38d3f9 100755 --- a/bin/que +++ b/bin/que @@ -109,7 +109,16 @@ Que.logger ||= Logger.new($stdout) if options.ci require "active_record" + ActiveRecord::Base.establish_connection( + adapter: "postgresql", + host: ENV.fetch("PGHOST", "localhost"), + user: ENV.fetch("PGUSER", "postgres"), + password: ENV.fetch("PGPASSWORD", ""), + database: ENV.fetch("PGDATABASE", "que-test"), + ) + Que.connection = ActiveRecord + Que.migrate! end begin