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 mysql support and enhance features #62

Open
wants to merge 31 commits into
base: master
Choose a base branch
from

Conversation

jawabuu
Copy link

@jawabuu jawabuu commented Jul 5, 2019

This pull request adds the following;

  • Ability to switch between postgres and mysql
  • Ability to use a remote database e.g. Amazon, CloudSQL e.t.c
  • Simplifies bringing up the project
  • Cleans up WORKDIR omissions in Dockerfile
  • Adds migrations in django compose command
  • Maintains functionality of the original repo

@jawabuu
Copy link
Author

jawabuu commented Jul 5, 2019

@dbader, @DahlitzFlorian.
Take a look.

@DahlitzFlorian
Copy link
Contributor

Hey @jawabuu,
first of all thank you for your interest and your contribution! In my personal opinion this PR shouldn't be merged. The repository corresponds to an article, which tries to explain the concept of containerizing a Django application using Docker. As this article was written for beginners, the article as well as the code base is kept to be as simple as possible.
You suggest a few enhancements, which are nice to know, but make it harder for beginners to understand it (at least in my personal opinion).
My suggestion: Keep your repository public and share it with others. It's an example of how to enhance what is provided by this repository and by the corresponding article. But as I said, I wouldn't merge it into RPs code base.

Nevertheless, I'm curious what @dbader thinks about it.

@jawabuu
Copy link
Author

jawabuu commented Jul 5, 2019

@DahlitzFlorian This is well received.
The PR is a direct result of me using this repo which is excellent to introduce beginners to docker.
The biggest challenge I tried to address was why a simple docker-compose up - d did not start the app.
This boiled down to addressing the database layer, migrations and the WORKDIR in the Dockerfile.
I believe these changes can be implemented without majorly affecting the original code.
On the advanced features, I am open to feedback on how I can make these more accessible to beginners.
I am currently testing these out myself.
Ultimately, if we can achieve docker-compose up -d and have the app run while staying true to the article but also optionally allowing beginners to grasp and explore advanced features, I believe it would make for a richer tutorial.

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.

4 participants