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

[BUG] ENV['APP_ENV'] isn't passed along to the server #3

Open
sdubinsky opened this issue Aug 30, 2022 · 6 comments
Open

[BUG] ENV['APP_ENV'] isn't passed along to the server #3

sdubinsky opened this issue Aug 30, 2022 · 6 comments

Comments

@sdubinsky
Copy link

sdubinsky commented Aug 30, 2022

Whatever I set ENV['APP_ENV'] to, when I run it with rackup directly, the server always starts in development mode. I would expect it to respect the env var, as it does when running it directly. This is on Ubuntu with ruby 3.0.1 and

    rack (3.0.0.beta1)
    rackup (0.2.2)
      rack (>= 3.0.0.beta1)
      webrick
    webrick (1.7.0)

My config.ru is

run do |env|
  [200, {}, ["Hello World"]]
end

What I mean is, when I run APP_ENV=production rackup config.ru I get:

Rack::Handler is deprecated and replaced by Rackup::Handler
Puma starting in single mode...
* Puma version: 5.6.5 (ruby 3.0.1-p64) ("Birdie's Version")
*  Min threads: 0
*  Max threads: 5
*  Environment: development
*          PID: 561633
* Listening on http://127.0.0.1:9292
* Listening on http://[::1]:9292
Use Ctrl-C to stop

but when I run APP_ENV=production puma I get:

Puma starting in single mode...
* Puma version: 5.6.5 (ruby 3.0.1-p64) ("Birdie's Version")
*  Min threads: 0
*  Max threads: 5
*  Environment: production
*          PID: 561750
* Listening on http://0.0.0.0:9292
Use Ctrl-C to stop

with the environment set to production as expected.

Honestly, I'm not sure whether the bug is in rack, rackup, or puma. If there's a better location for this, let me know. Thanks!

@ioquatix ioquatix reopened this Aug 31, 2022
@ioquatix
Copy link
Member

ioquatix commented Aug 31, 2022

Sorry, I thought this issue was created on the rack project, not rackup. This is the correct place to discuss this.

Uh, so I don't have a strong opinion about it, but I've never liked RACK_ENV and felt it was very confusing for users (it only supports development or deployment and doesn't really understand production).

Personally, I'd like to see us remove the handling of environments from rackup - I suspect what's happening here is that rackup is defaulting to RACK_ENV | development and forcing puma to consume that.

I think for Rack 3.0 release, we should make a 1.0 release of this which replicates the existing Rackup behaviour, but then I'd be totally fine with us cutting loose on a Rackup 2.0 release with a bunch of improvements and a rethink of what we want to provide (option handling, etc).

@sdubinsky
Copy link
Author

I suspect what's happening here is that rackup is defaulting to RACK_ENV | development and forcing puma to consume that.

This does seem to be what's happening:

deus-ex:~/code/puma_mwe$ RACK_ENV=production bundle exec rackup config.ru
Rack::Handler is deprecated and replaced by Rackup::Handler
Puma starting in single mode...
* Puma version: 5.6.5 (ruby 3.0.1-p64) ("Birdie's Version")
*  Min threads: 0
*  Max threads: 5
*  Environment: production
*          PID: 619341
* Listening on http://0.0.0.0:9292
Use Ctrl-C to stop

I think the current behavior is a bug, and I don't think you should preserve it in a 1.0 release. Puma expects the env to be something like production or development, not deployment.

@ioquatix
Copy link
Member

That seems reasonable to me, do you want to make a PR?

@sdubinsky
Copy link
Author

Sure.

Looking at the code, there doesn't seem to be one obvious place to do it. As far as I can tell, rackup is just passing all of its options to the handler, and counting on the handler to ignore whatever it doesn't need. I can see two possibilities:

  1. Rename environment to rack_environment in the rackup options. This has the advantage of being clearer with intent.
  2. add environment to the list of ignored options. This has the advantage of being a much smaller code change.

What do you think is best?

@ioquatix
Copy link
Member

ioquatix commented Sep 4, 2022

I think we should aim to remove RACK_ENV completely, so I think option (2) makes more sense.

@sdubinsky
Copy link
Author

I took a reasonable stab at it, and it's turning into a bit of a game of whack-a-mole trying to sort through everything. I have a workaround for my problem by running puma instead of rackup, and I think when you get around to removing RACK_ENV that will fix this anyway, so I'm OK with closing this.

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

No branches or pull requests

2 participants