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

Use the PORT environment variable for rails server #21267

Merged
merged 1 commit into from Aug 24, 2015

Conversation

Projects
None yet
4 participants
@davidcornu
Contributor

davidcornu commented Aug 17, 2015

Allow the server port to be set with the PORT environment variable.

  • This has become somewhat of a standard on various platforms
  • Would simplify setting a default port in development (which is currently only possible by either monkey-patching or using a custom script to start the server)
Show outdated Hide outdated railties/test/env_helpers.rb
@@ -21,6 +21,13 @@ def with_rack_env(env)
end
end
def with_port(port)

This comment has been minimized.

@senny

senny Aug 18, 2015

Member

can we wait with the helper extraction until it's needed in other cases as well. Once they're extracted it's harder to clean em up when tests get deleted or changed.

@senny

senny Aug 18, 2015

Member

can we wait with the helper extraction until it's needed in other cases as well. Once they're extracted it's harder to clean em up when tests get deleted or changed.

@senny

This comment has been minimized.

Show comment
Hide comment
@senny

senny Aug 18, 2015

Member

This will need an entry in the CHANGELOG as well.

Member

senny commented Aug 18, 2015

This will need an entry in the CHANGELOG as well.

@davidcornu

This comment has been minimized.

Show comment
Hide comment
@davidcornu

davidcornu Aug 18, 2015

Contributor

@senny done.

Contributor

davidcornu commented Aug 18, 2015

@senny done.

@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Aug 21, 2015

Member

Is not PORT too generic? I mean, in a machine with a lot of services running what PORT means?

Member

rafaelfranca commented Aug 21, 2015

Is not PORT too generic? I mean, in a machine with a lot of services running what PORT means?

@davidcornu
@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Aug 21, 2015

Member

😢 Heroku always creating weird conventions. cc @schneems

Member

rafaelfranca commented Aug 21, 2015

😢 Heroku always creating weird conventions. cc @schneems

@schneems

This comment has been minimized.

Show comment
Hide comment
@schneems

schneems Aug 21, 2015

Member

If Rails had set a convention before us, we would have followed it 😉

On Heroku it will help, but not that much. We already set this if you don't have a procfile: https://github.com/heroku/heroku-buildpack-ruby/blob/154fab366ff2a611893a825eff33d537d717a350/lib/language_pack/rails2.rb#L36.

I think we generally recommend against using $ rails s in production as it uses webrick by default. Though I guess you can specify $ rails s Puma, etc.

It looks like gunicorn for python supports $PORT: https://github.com/benoitc/gunicorn/blob/master/docs/source/2012-news.rst#0160--2012-11-19

All in all, it's a pretty simple patch and provide benefits. I'm 👍 . If issue arise from this I can own this minimal code path.

Member

schneems commented Aug 21, 2015

If Rails had set a convention before us, we would have followed it 😉

On Heroku it will help, but not that much. We already set this if you don't have a procfile: https://github.com/heroku/heroku-buildpack-ruby/blob/154fab366ff2a611893a825eff33d537d717a350/lib/language_pack/rails2.rb#L36.

I think we generally recommend against using $ rails s in production as it uses webrick by default. Though I guess you can specify $ rails s Puma, etc.

It looks like gunicorn for python supports $PORT: https://github.com/benoitc/gunicorn/blob/master/docs/source/2012-news.rst#0160--2012-11-19

All in all, it's a pretty simple patch and provide benefits. I'm 👍 . If issue arise from this I can own this minimal code path.

rafaelfranca added a commit that referenced this pull request Aug 24, 2015

Merge pull request #21267 from davidcornu/rails-server-port-env-var
Use the PORT environment variable for rails server

@rafaelfranca rafaelfranca merged commit 1c1ad2b into rails:master Aug 24, 2015

@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Aug 24, 2015

Member

I don't like environment variables changing behavior, but yeah... it is already a convention.

Member

rafaelfranca commented Aug 24, 2015

I don't like environment variables changing behavior, but yeah... it is already a convention.

@davidcornu davidcornu deleted the davidcornu:rails-server-port-env-var branch Aug 24, 2015

@davidcornu

This comment has been minimized.

Show comment
Hide comment
@davidcornu

davidcornu Aug 24, 2015

Contributor

🎉

Contributor

davidcornu commented Aug 24, 2015

🎉

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