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

Create docker image #313

Merged
merged 4 commits into from
Sep 29, 2023
Merged

Create docker image #313

merged 4 commits into from
Sep 29, 2023

Conversation

charalamm
Copy link
Collaborator

Added code for creating a docker image to be used when deploying the terracotta server. It has been used in an azure container app and it had really good results.

@dionhaefner
Copy link
Collaborator

Thanks for the PR.

What's the concrete use case for this? It looks like it's meant for production, but why use terracotta serve then instead of a real webserver with concurrency?

@charalamm
Copy link
Collaborator Author

charalamm commented Sep 8, 2023

@dionhaefner

What's the concrete use case for this? It looks like it's meant for production, but why use terracotta serve then instead of a real webserver with concurrency?

The intention is to use the docker image in a containerized deployment. I am going to use that image with azure container apps.
Good point about the real server. I added gunicorn with minimal configuration by default but not sure if it is enough. Do you mean servers like nginx?

@codecov
Copy link

codecov bot commented Sep 8, 2023

Codecov Report

Merging #313 (4a0ac35) into main (0cf1ea4) will not change coverage.
The diff coverage is n/a.

❗ Current head 4a0ac35 differs from pull request most recent head 16b485c. Consider uploading reports for the commit 16b485c to get more accurate results

@@           Coverage Diff           @@
##             main     #313   +/-   ##
=======================================
  Coverage   98.11%   98.11%           
=======================================
  Files          52       52           
  Lines        2338     2338           
  Branches      476      476           
=======================================
  Hits         2294     2294           
  Misses         29       29           
  Partials       15       15           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@dionhaefner dionhaefner left a comment

Choose a reason for hiding this comment

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

Thanks, this looks better now.

  1. Could we please move the makefile and dockerignore under a docker/ subfolder to keep this self-contained? (You might have to move the build context one level deeper)
  2. Please add a minimal example on how to use this to docker/README.md.

@charalamm
Copy link
Collaborator Author

  1. Could we please move the makefile and dockerignore under a docker/ subfolder to keep this self-contained? (You might have to move the build context one level deeper)
  2. Please add a minimal example on how to use this to docker/README.md.

Thanks @dionhaefner for the recommendations, I added them.

@dionhaefner
Copy link
Collaborator

LGTM, thanks

@dionhaefner dionhaefner merged commit 1a60e00 into main Sep 29, 2023
6 of 7 checks passed
@dionhaefner dionhaefner deleted the docker-image branch September 29, 2023 19:41
This pull request was closed.
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.

2 participants