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

Feat port setting #107

Merged
merged 4 commits into from
Jul 25, 2021
Merged

Feat port setting #107

merged 4 commits into from
Jul 25, 2021

Conversation

AbimbolaOO
Copy link
Collaborator

No description provided.

Copy link
Owner

@johnolafenwa johnolafenwa left a comment

Choose a reason for hiding this comment

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

Great work on this.
Please add back the EXPOSE 5000
While the expose command doesn't perform any function in a technical sense, the idea of it is that you can look at a Dockerfile and tell the port on which to run it without having to look at the source code of the app.

@AbimbolaOO
Copy link
Collaborator Author

Great work on this.
Please add back the EXPOSE 5000
While the expose command doesn't perform any function in a technical sense, the idea of it is that you can look at a Dockerfile and tell the port on which to run it without having to look at the source code of the app.

The reason I removed it was because It prevented me from setting Deepstack internal port successfully via the use of environmental variables. Can add back the line but comment it at the same time?

@OlafenwaMoses
Copy link
Collaborator

OlafenwaMoses commented Jul 16, 2021

Great work on this.
Please add back the EXPOSE 5000
While the expose command doesn't perform any function in a technical sense, the idea of it is that you can look at a Dockerfile and tell the port on which to run it without having to look at the source code of the app.

The reason I removed it was because It prevented me from setting Deepstack internal port successfully via the use of environmental variables. Can add back the line but comment it at the same time?

Don't comment it. Add it and leave it as is.

@johnolafenwa johnolafenwa merged commit b5652b6 into dev Jul 25, 2021
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.

3 participants