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

Remove builder-base and development image stages, run linting and tests outside Docker #111

Merged
merged 11 commits into from
Jul 19, 2022

Conversation

grahamalama
Copy link
Contributor

In this PR, we:

  • remove the development and builder-base stages of the Dockerfile so all that's left is a base stage for downloading Python dependencies and a production stage for running the application code. We still run the image in a "development" mode locally through the use of environment variables and overriding the image's CMD instruction.
  • run linting and testing locally (rather than through a Docker image)
  • add a .dockerignore so that only application code ends up in the production image

Closes #110

Now, we only have two Dockerfile stages:
- `base`, where we install dependencies with poetry
- `production`, where we copy the sources files and run the app

We use environment variables to run the app in a "nonprod" or "prod"
mode

We also ad a .dockerignore file to limit the files that make it into the
production container
@grahamalama grahamalama requested a review from a team as a code owner July 14, 2022 17:10
@grahamalama grahamalama temporarily deployed to cloudops July 14, 2022 17:11 Inactive
@grahamalama grahamalama temporarily deployed to cloudops July 14, 2022 17:14 Inactive
@grahamalama grahamalama temporarily deployed to cloudops July 14, 2022 20:04 Inactive
Copy link
Contributor

@leplatrem leplatrem left a comment

Choose a reason for hiding this comment

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

Wao 🤯 big shrink!

Weird, the missing PORT var is blocking the build on my env, and it wasn't caught in the Github Action build...

Should we have a Github Action that does a make start and curl --connect-timeout 30 --retry 300 --retry-delay 5 http://localhost:8000/ to make sure we don't break anything in docker-compose?

And I'm also facing #107, but I guess you don't, and it can be fixed in another PR.

$ make start
docker-compose up
/opt/homebrew/lib/python3.9/site-packages/paramiko/transport.py:236: CryptographyDeprecationWarning: Blowfish has been deprecated
  "class": algorithms.Blowfish,
Creating jira-bugzilla-integration_web_1 ... done
Attaching to jira-bugzilla-integration_web_1
web_1  | ERROR (catatonit:2): failed to exec pid1: Permission denied
jira-bugzilla-integration_web_1 exited with code 1

Random driveby: what would you think about prefixing makefile targets that go through Docker with container- or docker-?
eg. make start vs make docker-start

.dockerignore Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
config/local_dev.env Show resolved Hide resolved
@leplatrem
Copy link
Contributor

grahamalama deployed to cloudops 14 hours ago

I'm not familiar with this. What does it mean?

@grahamalama
Copy link
Contributor Author

grahamalama commented Jul 15, 2022

grahamalama deployed to cloudops 14 hours ago

I'm not familiar with this. What does it mean?

I'm also not really sure what this means -- I think it's because we have this configured in our Github actions? See this deployment / environment documentation.

- add `docker-` prefix for shell and start rules
- add a local start rule
- modify help text
@grahamalama grahamalama temporarily deployed to cloudops July 15, 2022 14:28 Inactive
@grahamalama
Copy link
Contributor Author

Random driveby: what would you think about prefixing makefile targets that go through Docker with container- or docker-?
eg. make start vs make docker-start

Good idea 849788e

@leplatrem
Copy link
Contributor

I'm #109 (comment) not really sure what this means -- I think it's because we have this configured in our Github actions? See this deployment / environment documentation.

I missed your previous comment.

I looks like we don't use these features. Even if that does not seem to affect anything, I would be in favor of removing anything that we don't use, and keep only the strict necessary and sufficient stuff

@grahamalama grahamalama merged commit 0fca04b into main Jul 19, 2022
@grahamalama grahamalama deleted the refactor-docker-setup branch July 19, 2022 12:40
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.

Remove development stage from Dockerfile
3 participants