IBM / template-node-typescript

Github Template that uses TypeScript with Node.js to create a BFF or Microservice API with Open API Specification
Apache License 2.0
71 stars 41 forks source link

Changes to fix the app.listen issue which fails the tekton pipeline at build stage #11

Closed RamyaRaghuveera closed 4 years ago

RamyaRaghuveera commented 4 years ago

These changes are currently in progress and not complete. This fixes the app.listen issue at the build stage but creates an error in the test stage of the pipeline. This pull request is created corresponding to issue https://github.com/IBM/template-node-typescript/issues/10 as suggested by Carlos

seansund commented 4 years ago

Is this PR ready for review? The description seems to indicate otherwise. If it isn't ready please change this PR to a draft

seansund commented 4 years ago

I fixed this issue just now in my fork by updating the Dockerfile and adding a .dockerignore

Dockerfile

FROM registry.access.redhat.com/ubi8/nodejs-12:1-52 AS builder

WORKDIR /opt/app-root/src

COPY . .

RUN ls -lA && npm install
RUN npm run build

FROM registry.access.redhat.com/ubi8/nodejs-12:1-52

COPY --from=builder /opt/app-root/src/dist dist
COPY --from=builder /opt/app-root/src/public public
COPY --from=builder /opt/app-root/src/package.json .
RUN npm install --production

ENV HOST=0.0.0.0 PORT=3000

EXPOSE 3000/tcp

CMD npm run serve

.dockerignore

.git
.github/
.cache

node_modules/
Jenkinsfile
CODE_OF_CONDUCT.md
LICENSE
README.md
chart/
dist/
docs/
seansund commented 4 years ago

Hey @mjperrins I didn't approve the changes yet because I don't think they are correct. Also the description says "These changes are currently in progress and not complete" so I was trying to understand the status

RamyaRaghuveera commented 4 years ago

Hi @mjperrins @seansund, These changes are in progress and not yet complete. This fixes the app.listen issue at the build stage but creates an error in the test stage of the pipeline. This is corresponding to issue #10. I had created this PR beacuse @csantanapr had asked in our slack conversation in #Catalyst-project channel to create PR as it would be easier to diff the changes and review. Sorry for the confusion. Could you please revert back these changes for now.

seansund commented 4 years ago

Hi @mjperrins @seansund, These changes are in progress and not yet complete. This fixes the app.listen issue at the build stage but creates an error in the test stage of the pipeline. This is corresponding to issue #10. I had created this PR beacuse @csantanapr had asked in our slack conversation in #Catalyst-project channel to create PR as it would be easier to diff the changes and review. Sorry for the confusion. Could you please revert back these changes for now.

@RamyaRaghuveera @mjperrins I will revert the changes. In the future please create a Draft PR until the change is ready to be merged