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

Allow overriding options of Configuration object #936

Merged
merged 1 commit into from Apr 1, 2016

Conversation

Projects
None yet
2 participants
@prathamesh-sonpatki
Contributor

prathamesh-sonpatki commented Mar 26, 2016

  • Currently it's not possible to override the default options for
    Puma::Configuration with user provided options.
  • I came across this issue while working on fixing server restart for
    Rails.
  • Rails can send it's own restart command to Puma and Puma should store
    it in it's configuration object. So that Puma::Launcher can use it.
  • After this patch it will be possible as user provided options will be
    taken into account in Configuration object.
@prathamesh-sonpatki

This comment has been minimized.

Show comment
Hide comment
@prathamesh-sonpatki

prathamesh-sonpatki Mar 26, 2016

Contributor

This partially fixes #907. We will need to make some changes on Rails side also to pass restart command to Puma. I am working on it.

@evanphx Please review.

Contributor

prathamesh-sonpatki commented Mar 26, 2016

This partially fixes #907. We will need to make some changes on Rails side also to pass restart command to Puma. I am working on it.

@evanphx Please review.

@evanphx

This comment has been minimized.

Show comment
Hide comment
@evanphx

evanphx Mar 28, 2016

Member

Could you please use 1.8 syntax for the arguments?

Member

evanphx commented Mar 28, 2016

Could you please use 1.8 syntax for the arguments?

@prathamesh-sonpatki

This comment has been minimized.

Show comment
Hide comment
@prathamesh-sonpatki

prathamesh-sonpatki Mar 30, 2016

Contributor

@evanphx Done. Also added a test case.

Contributor

prathamesh-sonpatki commented Mar 30, 2016

@evanphx Done. Also added a test case.

Allow overriding options of Configuration object
- Currently it's not possible to override the default options for
  Puma::Configuration with user provided options.
- I came across this issue while working on fixing server restart for
  Rails.
- Rails can send it's own restart command to Puma and Puma should store
  it in it's configuration object. So that Puma::Launcher can use it.
- After this patch it will be possible as user provided options will be
  taken into account in Configuration object.
@prathamesh-sonpatki

This comment has been minimized.

Show comment
Hide comment
@prathamesh-sonpatki

prathamesh-sonpatki Mar 30, 2016

Contributor

I have also opened PR rails/rails#24331 on Rails to pass the required restart_cmd to Puma.

Contributor

prathamesh-sonpatki commented Mar 30, 2016

I have also opened PR rails/rails#24331 on Rails to pass the required restart_cmd to Puma.

prathamesh-sonpatki added a commit to prathamesh-sonpatki/rails that referenced this pull request Mar 30, 2016

Fix rails restart issue with Puma
- We need to pass the restart command to Puma so that it will use it
  while restarting the server.
- Also made sure that all the options passed by user while starting
  the server are used in the generated restart command so that they will
  be used while restarting the server.
- Besides that we need to remove the server.pid file for the previous running
  server because otherwise Rack complains about it's presence.
- We don't care if the server.pid file does not exist. We only want to delete
  it if it exists.
- This also requires some changes on Puma side which are being tracked
  here - puma/puma#936.
- Fixes rails#23910.

@evanphx evanphx merged commit fc12a4e into puma:master Apr 1, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@prathamesh-sonpatki

This comment has been minimized.

Show comment
Hide comment
@prathamesh-sonpatki

prathamesh-sonpatki Apr 2, 2016

Contributor

Thanks @evanphx. Will you cut a new release so that Rails can depend on that version. The changes required to do on Rails side are already merged - rails/rails#24331

Contributor

prathamesh-sonpatki commented Apr 2, 2016

Thanks @evanphx. Will you cut a new release so that Rails can depend on that version. The changes required to do on Rails side are already merged - rails/rails#24331

@prathamesh-sonpatki prathamesh-sonpatki deleted the prathamesh-sonpatki:allow-overriding-config-options branch Apr 2, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment