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

GH-268 #269

Closed
wants to merge 14 commits into from
Closed

GH-268 #269

wants to merge 14 commits into from

Conversation

padcom
Copy link

@padcom padcom commented Sep 26, 2012

Pull request with changes implementing the ability to declare multiple PORTs. See examples. Implements #268

@padcom
Copy link
Author

padcom commented Sep 26, 2012

The general idea in this implementation is to stop treating PORT as some special case and include it in a copy of the engine.env (where appropriate) thus making room for additional automatic variables. The same goes for both the execution as well as exports to other systems.

@ddollar
Copy link
Owner

ddollar commented Jan 14, 2013

I'm torn on this feature. The idea seems decent, but I'm hesitant to create a new standard (PORT1, PORT2, etc) without careful consideration.

Is it possible to do the PORT math inline or in a wrapper script?

@padcom
Copy link
Author

padcom commented Jan 14, 2013

I know what you mean. I've been going over 2 different approaches:

  1. Use inline calculation of "the next port"
  2. Use PORT in conjunction with PORTx

I went with the first option to keep it backwards-compatible. Imagine a situation like this:

example: run.sh --listen PORT --serve PORT

In this case it is quite obvious we need 2 different ports.

example: run.sh --listen PORT --verify PORT

In this case the --verify might mean that we need the same port. Sure we might suggest that it makes no sense but if you take the current behavior it is exactly what would happen: the same port number would be passed on twice. That leaves with another choice:

  1. Keep it backwards compatible and extend the API to allow for additional PORTx keeping the original one for backwards compatibility
  2. Forget about the backwards compatibility and extend how the ports are being passed on to the application.

BTW this feature in its current form keeps the PORT variable intact meaning it is an extension rather than a completely new standard. That way we can kill two birds with one stone, in a matter of speaking.

HTH

@ddollar
Copy link
Owner

ddollar commented May 3, 2013

I think I'm going to err on the side of simplicity for now and say that this shouldn't be part of foreman. I really do appreciate the effort you put into this pull request.

@ddollar ddollar closed this May 3, 2013
@padcom
Copy link
Author

padcom commented May 3, 2013

I understand it is controversial to put it in the core of Foreman. Do you see any other option, like a plugin or something, that this functionality could exist in? I hate to create a fork just to include this feature as this makes maintaining it an unnecessary burden.

@ddollar
Copy link
Owner

ddollar commented May 3, 2013

I would solve this using binscripts.

# Procfile
example: bin/example

# bin/example
#!/bin/sh
SERVE=$(expr $PORT + 50)
run.sh --listen $PORT --serve $SERVE

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