-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Dockerized version of the application #66
base: master
Are you sure you want to change the base?
Changes from all commits
713bc8f
d01a7f5
8269f4d
0c490b5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
/public/vendor | ||
/public/images |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
v12.18.4 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
FROM node:12.18.4-alpine | ||
|
||
LABEL maintainer="[email protected] || [email protected]" | ||
LABEL author="Roger Kondrat" | ||
|
||
RUN mkdir -p /src/app/node_modules && chown -R node:node /src/app | ||
WORKDIR /src/app | ||
|
||
|
||
COPY package*.json ./ | ||
RUN npm install --silent | ||
|
||
COPY . ./ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The way I usually do this step is by individually copying the relevant top-level files or directories. In this case, it'll probably cause problems if the student did an "npm install" before building the image. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you give me a more specific suggestion? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we only need
|
||
|
||
ENV PORT=5000 | ||
|
||
EXPOSE 5000 | ||
|
||
CMD [ "npm", "run", "local" ] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
version: '3.8' | ||
|
||
services: | ||
node-express: | ||
image: rogerkondrat/tweeter | ||
container_name: tweeter | ||
build: | ||
context: ./ | ||
dockerfile: Dockerfile | ||
ports: | ||
- "5000:5000" | ||
- "3000:3000" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why port 3000? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Quoted from above: Like all things weird I had a problem once and it was a while ago and it fixed it. Sometimes when I copy and paste across most of my existing processes I forget to delete something. Thanks for pointing this out. Also I just wrote this quickly. I never thought it would go to production as is, so no judging please ;) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No problem! That's why we do code reviews ;) |
||
volumes: | ||
- .:/src/app | ||
- /src/app/node_modules | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure why you make node_modules a volume? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Like all things weird I had a problem once and it was a while ago and it fixed it. Sometimes when I copy and paste across most of my existing processes I forget to delete something. Thanks for pointing this out. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd run
npm ci
here instead; it'll install exactly the versions used in package-lock.json.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a great point Emile.