WIP: Consolidate dockerfiles #462
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vasconsaurus
force-pushed
the
4921-consolidate-dockerfiles
branch
2 times, most recently
from
cc37481
to
937e638
Compare
vasconsaurus
force-pushed
the
4921-consolidate-dockerfiles
branch
from
937e638
to
0af6f03
Compare
trying to find common ground between dockerfiles
Removed/changed – environment variables:
- MIN_INSTANCES=4 - Seems to have been used for passenger.
This is hardcoded in docker-entrypoint.sh (old start.sh) for puma
- MAX_POOL_SIZE=4 – Same as above.
- TERM=xterm – Seems to have been used with phantomjs
- PRODUCT=pender – We are unsure. We are keeping only APP.
- SERVER_PORT=8000 – We are moving this out.
– DEPLOYUSER=pender – We are keeping only APP.
- DEPLOYDIR=/app/pender – Change to ARG DIRPATH (it's used for other environments as well)
Removed/Changed
- RUN chown -R ${DEPLOYUSER}:www-data ${DEPLOYDIR}: Seems to have been used
when we were deploying with nginx.
- RUN echo "gem: --no-rdoc --no-ri" > ~/.gemrc: Deprecated.
- MAINTAINER [email protected]: Deprecated.
Talked to Caio: - He thinks we can hardcode this in config - We also don't think profile.rb is necessary anymore
vasconsaurus
force-pushed
the
4921-consolidate-dockerfiles
branch
2 times, most recently
from
88ad39f
to
2a40a62
Compare
Now both docker build + run and docker compose build + compose up work Ran the tests and only one crowdtangle test was failing.
graphicsmagick: - we seem to have added `imagemagick` when we were using `chromeshot`: [here](4f97370). - and then to fix tests(?) we updated to `graphicsmagick`: [here](2813f7e) inotify-tools: - was removed: [commit](08746f8) python: - looks like it might have been used for youtube-dl: [commit](bde6bb4)
also renamed files
removed environment variables not used during build time add needed environment variables to docker-compose
Now we have one entrypoint file: - pender local and deployed - pender-background local and deployed
seems like it shouldn't have been in the production folder and doesn't seem to be used anywhere
vasconsaurus
force-pushed
the
4921-consolidate-dockerfiles
branch
3 times, most recently
from
ea40476
to
08c5bdf
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
trying to find common ground between dockerfiles
References: 4921, 5020
How has this been tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can verify the changes. Please describe whether or not you implemented automated tests.
Things to pay attention to during code review
Please describe parts of the change that require extra attention during code review, for example:
Checklist