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

Support PORT from environment variable #3

Closed
wants to merge 1 commit into from

Conversation

ronakbanka
Copy link

Why

As of now there is no way to specify port using environment variable , which is used for most of cloud platform , eg heroku , cloudfoundry.

Changes

  • Provision to use PORT from environment variable.
  • Add UI message for server start along with PORT in use

@mtojek
Copy link
Owner

mtojek commented Dec 16, 2016

Hi Ronak,

thank you for contributing. Unfortunately I can't accept this PR because you simply ignored original command line arg because of introducting PORT env variable. That's not how it works.

Btw. you can still run the app binary like this and I'm pretty sure that's doable on Heroku ;) :

$ export PORT="9002"
$ greenwall -hostPort ":$PORT"

I'll close this PR and leave a issue for an alternative config source.

@mtojek mtojek closed this Dec 16, 2016
@ronakbanka
Copy link
Author

simply ignored original command line arg because of introducting PORT env variable

If env variable is not defined , this will still respect commandline hostPort

if len(serverPort) == 0 {
		addr = httpServer.applicationConfiguration.HostPort
	}

case 1. when env port is defined

[ronak.banka greenwall (support_env_port)]$ PORT=8081 greenwall
Start listening on :8081

case 2. when env port is not defined

[ronak.banka greenwall (support_env_port)]$ greenwall
Start listening on :9001

case 3. when port is defined using cli flag

[ronak.banka greenwall (support_env_port)]$ greenwall -hostPort ":8082"
Start listening on :8082

you can still run the app binary like this and I'm pretty sure that's doable on Heroku

greenwall -hostPort ":$PORT"

This will work, without need to export

There should be a switch in command line args which will force the application to read config entirely from env vars.( from issue)

I haven't seen this practice for forcing environment var load by using additional flag, can you point to some project which uses this practice for golang based cli's?

@mtojek
Copy link
Owner

mtojek commented Dec 16, 2016

@ronakbanka I abandoned the idea of additional switch for env. Now ENV variables have the priority. Fixed here: #5

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

Successfully merging this pull request may close these issues.

None yet

2 participants