search

nickstenning / honcho

Honcho: a python clone of Foreman. For managing Procfile-based applications.
http://pypi.python.org/pypi/honcho
MIT License
1.59k stars 145 forks source link

After honcho 0.6 port should be a multiple of 1000 #132

Closed playpauseandstop closed 9 years ago

playpauseandstop commented 9 years ago

I used honcho for a while and liked it ability to use custom base ports, so its no need to start base ports from multiple of 1000 as in foreman.

But after 0.6 release this changed and now I got something like:

PORT=8110 . venv/bin/activate && honcho start web -f ec2Procfile
. venv/bin/activate && honcho start worker
Traceback (most recent call last):
  File "/home/playpauseandstop/Projects/.../.../venv/bin/honcho", line 11, in <module>
    sys.exit(main())
  File "/home/playpauseandstop/Projects/.../.../venv/lib/python2.7/site-packages/honcho/command.py", line 269, in main
    COMMANDS[args.command](args)
  File "/home/playpauseandstop/Projects/.../.../venv/lib/python2.7/site-packages/honcho/command.py", line 212, in command_start
    port=port):
  File "/home/playpauseandstop/Projects/.../.../venv/lib/python2.7/site-packages/honcho/environ.py", line 114, in expand_processes
    assert port % 1000 == 0, "port must be multiple of 1000"
AssertionError: port must be multiple of 1000

when try to start honcho on 8110 port.

Do we really need mimicrify foreman here? Or can continue live with custom base ports?

playpauseandstop commented 9 years ago

As written in https://github.com/ddollar/foreman/issues/521 there is no strict requirement for base port to be a multiple of 1000

Here the source code in Ruby for getting port for a given process https://github.com/ddollar/foreman/blob/fbf32e2b64e436e78ad627330187565a5eacf779/lib/foreman/engine.rb#L253

So I suggest that AssertionError should be removed from honcho as well

slafs commented 9 years ago

Hey! Thanks for the report.

You're probably right and I'm surprised with this assertion as well. Maybe @nickstenning can tell us why did he put it there. Made a quick fix in PR #133 which removes the assertion.

jpadilla commented 9 years ago

This bit me too.

diwu1989 commented 9 years ago

Wow, how did this even make it into the codebase? I feel like this is one of those things that common sense says, why would we have to limit the ports to be multiples of 1000?