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

Add to production sample #132

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Add to production sample #132

wants to merge 8 commits into from

Conversation

lorensr
Copy link
Contributor

@lorensr lorensr commented Apr 13, 2022

  • Add Dockerfile and shared connectionOptions
  • Add HTTP server to provide status (currently responds to any route)
  • Don't copy certs to image on build

Adding instrumentation and more README updates in next PR. And then automated cloud test.

COPY ["package.json", "./"]
RUN npm install --production

ARG TEMPORAL_SERVER="host.docker.internal:7233"
Copy link
Member

Choose a reason for hiding this comment

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

Why put these in args?
It's typically something you'd provide at container run time, not bake into the image since this image could be used in different environments.

Copy link
Member

@bergundy bergundy left a comment

Choose a reason for hiding this comment

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

Not loving this sample.
It's not a complete production sample because it omits logging / metrics setup and has a lot of duplication with the mtls sample.

@lorensr lorensr self-assigned this May 11, 2022
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