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

Added heroku integration in readme #222

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mayukh18
Copy link

PR for #187

Copy link
Member

@nirs nirs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mayukh18 this is a good start, thanks!

I think we need more functionality. I updated the issue to add the missing details. Do you think you can add the missing functionality?

"name": "ROSE",
"description": "ROSE project car race game.",
"repository": "https://github.com/RedHat-Israel/ROSE",
"keywords": ["race", "game", "python"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should also describe:

  • The server is started by running the rose-server script
  • Port 8880 should be open to receive web clients
  • Port 8888 should be open to receive rose-client(s)

See the updated issue for more details.

Copy link
Member

@cben cben Oct 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, tried the button now from your fork https://github.com/mayukh18/ROSE/ => https://cben-rose-test.herokuapp.com, says "Application Error", heroku logs show:

2017-10-21T17:31:29.442324+00:00 heroku[router]: at=error code=H14 desc="No web processes running" method=GET path="/" host=cben-rose-test.herokuapp.com request_id=68b48dde-386d-4756-a4bd-a5901ea49fe5 fwd="141.226.169.57" dyno= connect= service= status=503 bytes= protocol=https

https://cben-rose-test.herokuapp.com:8880/ doesn't work either, connection refused.

  • I don't see anything in https://devcenter.heroku.com/articles/app-json-schema or elsewhere about specifying what port server internally listens on; we have to respect PORT env var that heroku provides.

  • On the outside, heroku router only listens on http[s] :80 and :443. That's fine for user interface, maybe will need to document that :8880 is not needed with heroku.

  • The nastier implication is you can't have 2nd outside port for rose-client. The client will have to connect on 80/443, and will need another mechanism for server to differentiate connection type (URL path? content-type? user-agent (meh)? custom header?).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I too didn't find any documentation related to servers and ports in https://devcenter.heroku.com/articles/app-json-schema and related articles. Second thing which I know is true is that heroku pre-assigns $PORT. What we need is a Procfile

"description": "ROSE project car race game.",
"repository": "https://github.com/RedHat-Israel/ROSE",
"keywords": ["race", "game", "python"]
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Newline missing here.

@nirs
Copy link
Member

nirs commented Oct 21, 2017

Without a way to open a port for the client, Heroku is useless for us unless we redesign the way the client communicates with the server.

I think we should stop this effort for now, and try OpenShift, it may be more flexible.

@mayukh18
Copy link
Author

Yes without a configurable port it can't be used properly.

@nirs
Copy link
Member

nirs commented Oct 23, 2017

@mayukh18 see #239, with this we should be able to deploy on Heroku.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants