drone / drone-wall

Dashboard for the Drone CI server
260 stars 44 forks source link

Rename port envvar #34

Open kpacha opened 8 years ago

kpacha commented 8 years ago

Do not use the $PORT env var because it doesn't allow you to use drone-wall in mesos environments.

Tathanen commented 8 years ago

In the readme there, you've replaced port with wall_port but in actuality, that first port was referring to api_port. It's kind of a confusing sentence, all of the vars there are implicitly prefixed by api_. The port->wall_port change you're making is for a var that was never mentioned in the readme in the first place, so you'll need to add a new sentence just for it (and maybe just add the api_ bits to that list of vars for clarity).

@scottferg would you mind vetting the actual code changes here? I'm not Docker-savvy enough to really be able to parse most of it. For context, this is being added to a separate branch for the v2 version, I'm not sure if the :2.1 in there relates to any github release notation tho.

flyinprogrammer commented 8 years ago

Also @kpacha if you're using marathon to do docker deploys on mesos then this change isn't actually required. You just have to know about some marathon wizardry whereby when you setup the port ask in your POST to marathon, set containerPort = 0; then marathon with expose, and set PORT to the ephemeral port it finds on the host:

From the docs:

However, if you have set containerPort to 0 then this will be the same as hostPort and you can use the $PORT environment variables.

scottferg commented 8 years ago

LGTM

Tathanen commented 8 years ago

@kpacha if you wanna make those doc updates (that table one in particular) I'll get this merged in.