Using variable expansion in POSIX shell to provide default values

[jjmaestro]

I found it convenient to be able to change the HOST (as well as the PORT) and since an empty $HOST would fail, I thought of providing default values like in shell scripting (since, from what I see, the bundle will exec that line in the shell).

Is this the "proper way" to do this with foreman?

Thanks!

[obfuscurity]

@gorsuch You have any input on this? It looks very anti-Ruby-ish, but perhaps you've seen something similar before.

[jjmaestro]

@obfuscurity that's what I thought too :-/ But "It Works On My Machine (TM)" :D And I needed a way to force launching tasseo in localhost and not 0.0.0.0.

I probably missed something... I am definitely open to suggestions (and, of course, the Ruby way of doing it :)

[jjmaestro]

@gorsuch Any ideas? I guess a better way would be to just add the -o $HOST and expect the user to configure a proper .env file with HOST=127.0.0.1 so that foreman parses it when starting bundle :-?

[jjmaestro]

I hope this approach is more ruby-friendly :) I think it's cleaner.

I've also updated the doc in the README file, in case it gets merged it will avoid a lot of confusion!

[gorsuch]

@jjmaestro hey, sorry for my late reply!

My only concern with any of these changes is whether it will work on Heroku out of the box or not. I didn't mind the shell expansion work at all, as long as things work without modification on Heroku.

Have you been able to test that?

[jjmaestro]

No worries! :) We are all very busy and this is Open Source! :D

I haven't tried it on Heroku. But the env file approach is not only cleaner but I'm pretty sure it should work there as well (from what I understand, it's how you are supposed to work with foreman).

Also, from what I see in https://devcenter.heroku.com/articles/config-vars it works. You can see in my pull request that I've modified the README and added the lines to configure the HOST env variable in Heroku... so I think anyone following the instructions should get it up and running without a problem.

What do you think? merge it maybe? :D

Thanks!

[jjmaestro]

After reading the Heroku doc carefully, I found this:

Your web process loads on port 5000 because this is what Foreman provides as a default in the $PORT env var. It’s important that your web process respect this value, since it’s used by the Heroku platform when you deploy.

Thus, I'm removing the line from the README that showed how to set PORT to (potentially) another value in the Heroku environment.

[obfuscurity]

I feel like a dick now that you've put so much time into this PR, but I have a couple concerns.

First, it requires manual intervention by the user to create an .env file. This bugs me in that a feature needed by <1% of the user base suddenly causes compatibility problems for the other >99% that will never use it.

Second, Heroku users shouldn't be required to set HOST. I think this would cause a lot of confusion and/or users asking WAT with us trying to justify the decision.

Lastly, and this is purely subjective, but it feels like an antipattern to me. I've never seen anyone else do this. Which doesn't mean it isn't a valid concern for you, but it seems to reinforce the notion that this is an edge case that doesn't merit merging upstream.

[jjmaestro]

Well, don't feel like that! It's just a welcomed informed opinion, and quite a good one at that :)

After reading your comment, I actually agree with it completely. I've been thinking about how to deal with this and I was not very happy with my overall approach (plus I'm quite new to Ruby so I am quite uncertain as what are the good patterns and deploy procedures).

I've come to believe it's actually "a problem" with foreman: why does it set PORT to 5000 by default but leaves HOST unset when it could/should default to either 127.0.0.1 or 0.0.0.0, depending on your "security" considerations?. If foreman were to set HOST to some value, we could leave the Procfile line with the -o $HOST without breaking anything out-of-the-box and allowing users like me to easily set it to a different value without having to patch upstream.

So, all in all, having to set .env is, as you said, requires user intervention and it's a bit cumbersome.

Since it's just a little change that is probably easily maintained as a patch in my setup, I'll go with that route (although is not so nice when having to patch a gem / installed package, I'll live with it :) But I might reach out to @ddollar and see if foreman can handle this by default :D I might be missing something, though... it seems weird it's so hard / uncommon for people to want to run things just in localhost as opposed to binding all available interfaces :-?

Anyway, thanks a lot to both of you for your comments and help!!

Cheers,

[obfuscurity]

:sparkles: