Skip to content

Conversation

@tschafer-gc
Copy link
Contributor

WEBrick appears to be approaching EOL and is being removed from Rackup, as mentioned here. This is causing issues with a number of services as discussed here. By switching to Puma we can solve this issue while also still supporting both Rack 2 and 3.

@tschafer-gc tschafer-gc force-pushed the tschafer-switch-to-puma branch from 1f6c099 to 1702563 Compare November 8, 2024 16:24
@tschafer-gc tschafer-gc force-pushed the tschafer-switch-to-puma branch from 1702563 to a4b5040 Compare November 8, 2024 17:28
@tschafer-gc tschafer-gc force-pushed the tschafer-switch-to-puma branch from 5dcbee3 to 529392f Compare November 8, 2024 22:58
@tschafer-gc tschafer-gc marked this pull request as ready for review November 13, 2024 09:58
@stephenbinns
Copy link
Contributor

Looks good but can we update the smoke test so it runs both Rack 2 and 3 - https://github.com/gocardless/que/blob/master/.github/workflows/tests.yml#L22-L57

@tschafer-gc tschafer-gc force-pushed the tschafer-switch-to-puma branch 2 times, most recently from 8523a09 to bca0dd5 Compare November 14, 2024 10:13
@tschafer-gc
Copy link
Contributor Author

Looks good but can we update the smoke test so it runs both Rack 2 and 3 - https://github.com/gocardless/que/blob/master/.github/workflows/tests.yml#L22-L57

@stephenbinns I've just updated the smoke tests - does that look okay? My implementation feels a bit hacky but I'm not sure of a better way to ensure it uses both Rack versions

@tschafer-gc tschafer-gc force-pushed the tschafer-switch-to-puma branch from bca0dd5 to 5fcf274 Compare November 14, 2024 10:23
@stephenbinns
Copy link
Contributor

stephenbinns commented Nov 14, 2024

Looks good but can we update the smoke test so it runs both Rack 2 and 3 - https://github.com/gocardless/que/blob/master/.github/workflows/tests.yml#L22-L57

@stephenbinns I've just updated the smoke tests - does that look okay? My implementation feels a bit hacky but I'm not sure of a better way to ensure it uses both Rack versions

Makes sense, as for making it a bit less hacky you should be able to put a conditional in the Gemfile to achieve the same result

if ENV['RACK_VERSION'] == '2.0'
  gem "rack", "~> 2.0"
else
  gem "rack", "~> 3.0"
end

And then pass the context in the env for the github action

    env:
      RACK_VERSION: "${{ matrix.rack-version }}"

@tschafer-gc tschafer-gc force-pushed the tschafer-switch-to-puma branch 2 times, most recently from 31ac735 to b479947 Compare November 14, 2024 11:58
Gemfile Outdated
end

gem "rack", ENV['RACK_VERSION']
if ENV['RACK_VERSION'] == '2.0'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Versions can be compared using: https://stackoverflow.com/a/3064161

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, updated - thanks!

@tschafer-gc tschafer-gc force-pushed the tschafer-switch-to-puma branch from b479947 to 0063b34 Compare November 14, 2024 12:06
@tschafer-gc tschafer-gc force-pushed the tschafer-switch-to-puma branch 3 times, most recently from 3717bc3 to aa62ef9 Compare November 14, 2024 15:38
@tschafer-gc tschafer-gc force-pushed the tschafer-switch-to-puma branch from aa62ef9 to a2c331f Compare November 14, 2024 16:13
@tschafer-gc tschafer-gc merged commit 2487508 into master Nov 18, 2024
17 checks passed
@tschafer-gc tschafer-gc deleted the tschafer-switch-to-puma branch November 18, 2024 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants