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

Specified parameters should overwrite loaded config files #1203

Merged
merged 1 commit into from Feb 9, 2017

Conversation

twalpole
Copy link
Contributor

Fix issue #1199, #1200, #1201 by allowing specified parameters to overwrite those loaded from config files.

@pudiva
Copy link

pudiva commented Feb 1, 2017

this was already fixed on v3.6.2 and not merged on v3.7.0 for some reason.

@pudiva
Copy link

pudiva commented Feb 1, 2017

see #1200

@twalpole
Copy link
Contributor Author

twalpole commented Feb 1, 2017

@ayghor It wasn't "fixed" on v3.6.2 - a fix for a different issue was reverted on 3.6.2 because while it fixed the previous issue it had created this new issue. So after reverting the previous issue still existed. This PR fixes both issues.

@pudiva
Copy link

pudiva commented Feb 2, 2017

oops sry for that <3

@twalpole
Copy link
Contributor Author

twalpole commented Feb 2, 2017

@nateberkopec Thoughts?

@nateberkopec
Copy link
Member

Wonderful, thanks @twalpole. Taking a look.

@bikramwp
Copy link

bikramwp commented Mar 2, 2017

In Rails 5.0.1, this change looks like it causes the options in config/puma.rb to be ignored:

Scenario 1: config/puma.rb with custom bind option, running rails s command

config/puma.rb contains:

bind ENV.fetch("BIND") { "tcp://127.0.0.1:4444" }

In Puma 3.7.0, that generated an options array of:

[
  { ... , :Port=>3000, :Host=>"localhost"  },
  { ... , :binds=>["tcp://localhost:3000"] },
  { ... , :binds=>["tcp://127.0.0.1:4444"] }
]

In Puma 3.7.1, it generates:

[
  { ... , :binds=>["tcp://127.0.0.1:4444"] },
  { ... , :Port=>3000, :Host=>"localhost"  },
  { ... , :binds=>["tcp://localhost:3000"] }
]

Scenario 2: config/puma.rb with custom bind option, running rails s -p 9999 command

config/puma.rb: contains (same as Scenario 1):

bind ENV.fetch("BIND") { "tcp://127.0.0.1:4444" }

In Puma 3.7.0, that generated an options array of:

[
  { ... , :Port=>9999, :Host=>"localhost"  },
  { ... , :binds=>["tcp://localhost:9999"] },
  { ... , :binds=>["tcp://127.0.0.1:4444"] }
]

In Puma 3.7.1, it generates:

[
  { ... , :binds=>["tcp://127.0.0.1:4444"] },
  { ... , :Port=>9999, :Host=>"localhost"  },
  { ... , :binds=>["tcp://localhost:9999"] }
]

So, Rails' config (internal default or specified on rails s command) is always taking precedence
and config/puma.rb settings don't take effect.

I think this pull request is right in that rails s options should take priority over config/puma.rb,
but in the case where no rails s options are specified on the command line, config/puma.rb should
take precedence over Rails' internal defaults.

Unfortunately, I am not sure what the proper solution would be: is Rails configuring Puma incorrectly
or should Puma be working differently?

@nateberkopec
Copy link
Member

@bikramwp see #1229.

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

Successfully merging this pull request may close these issues.

None yet

4 participants