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

Adding Docker-Compose #53

Merged
merged 16 commits into from
Jun 30, 2020
Merged

Adding Docker-Compose #53

merged 16 commits into from
Jun 30, 2020

Conversation

abhinavtripathy
Copy link
Member

@abhinavtripathy abhinavtripathy commented Jun 30, 2020

Added docker-compose with rails and postgres containers. Added shell scripts to access both containers. Added Dockerfile with yarn support and added db_setup script. Added documentation to readme for building and running containers with common errors. This closes #17

@abhinavtripathy abhinavtripathy added backend dependencies Pull requests that update a dependency file deployment Issues related to deploying the application Development documentation Improvements or additions to documentation labels Jun 30, 2020
@abhinavtripathy abhinavtripathy added this to the Set Up milestone Jun 30, 2020
Copy link
Member

@Saakshaat Saakshaat left a comment

Choose a reason for hiding this comment

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

Looks good overall. Just some minor nitpicking about formatting and using env variables.

README.md Outdated
```
sudo bash docker_shell.sh
```
If you want to access the rails console, then run the following comand after the previous one:
Copy link
Member

Choose a reason for hiding this comment

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

This is definitely not a priority but could you fix the spelling on command?

volumes:
- ./:/usr/src/app
ports:
- "3000:3000"
Copy link
Member

Choose a reason for hiding this comment

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

Silly EOL error that might return to haunt us later.

Consider using a Ruby formatter in a .gitattribute file that will automatically handle this, along with other formatting issues, when the changes are pushed to our remote.

For Python, I use a PEP-8 formatter called Black; I found this for Ruby, maybe this'll be helpful in our case.

EXPOSE 3000

# Default command that runs when the container starts
CMD ["bash", "docker/docker_set_up.sh"]
Copy link
Member

Choose a reason for hiding this comment

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

EOL error again.

@@ -0,0 +1,3 @@
rm tmp/pids/server.pid
yarn install --check-files
bundle exec rails server -b 0.0.0.0
Copy link
Member

@Saakshaat Saakshaat Jun 30, 2020

Choose a reason for hiding this comment

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

@abhinavtripathy are we starting the server at 0.0.0.0 or does this reference the port specified in docker-compose.yml

Copy link
Member Author

Choose a reason for hiding this comment

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

it references the port specified in docker-compose

docker_shell.sh Outdated
@@ -0,0 +1 @@
docker-compose exec rails ${@:-/bin/bash}
Copy link
Member

Choose a reason for hiding this comment

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

EOL 💀

Copy link
Member

@Saakshaat Saakshaat left a comment

Choose a reason for hiding this comment

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

Looks good to go!

@abhinavtripathy abhinavtripathy requested a review from harsh183 June 30, 2020 06:39
@harsh183
Copy link
Collaborator

Looks great!!

@harsh183 harsh183 merged commit bbdb9bc into master Jun 30, 2020
@harsh183 harsh183 deleted the docker-refresh branch June 30, 2020 06:41
@harsh183 harsh183 mentioned this pull request Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend dependencies Pull requests that update a dependency file deployment Issues related to deploying the application Development documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setup Docker/Podman
3 participants