-
-
Notifications
You must be signed in to change notification settings - Fork 25
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
Use Docker for local development environment #44
Conversation
🎉 🎉
|
@billglover I tested it out by forking your fork into my personal account, and running I believe it'll be fixed if you pull in the latest master changes -- e.g. (I tried that on my personal fork and re-ran |
I'll give it a go shortly. Dockerfile does include the I can see there is a conflict with the README, but I'll have a go at addressing this. |
Update: actually (in an attempt to make it easier) I submitted a PR to your fork with the latest as well: https://github.com/billglover/django-concept/pull/1 (Thanks. And yeah, I noticed |
Update master branch with latest changes
I've merged the migrations, along with your PR. I've just run this on a clean machine and it seemed to stand-up CBv3 correctly. Would appreciate a couple of other people running through this and confirming it all works as expected. |
I’m not sure either are desirable but neither are the result of using Docker for local development. Static content does need to be addressed in that we need to make a decision on how this is served. |
@d3vild06 A belated thanks for testing this out!
That's a norm for Django, heh. One of the things that always bothered me. The project is called a certain name, and then we create an app inside it with the same name.
Ah, good pointing out -- I'll file an issue about adding some content to that Django home page to explain the relevant URLs after loading up the app -- namely |
When running on Docker Desktop for Windows we have reports of the following error message:
|
The documentation here indicates that you need to specify which drives you want to use with Docker in the settings of the Docker App. This appears to be unique to Windows and is probably a result of the way virtualisation is implemented. https://docs.docker.com/docker-for-windows/ To use volumes with Docker, users will need to explicitly share drives with the Docker App. For must users this will be the C drive but this is not guaranteed. |
@Bill @lpatmo @d3vild06 Django can accommodate this change, if desired, and there's no penualty, should we want to move everything up a directory. Typically you can remove the extra nesting with (in a venv/container config file) |
@billglover, per Windows setup, maybe it should be explicit--if confirmed through tests--that setting a separate directory from the |
Testing was performed on Windows 10 Pro with docker installed only based on the setup instructions. edit: |
@nuageklow are you able to include a copy of the error you are seeing in your comment so that I can make sure we address it. |
I think I’ve found something that might make it easier for Windows users. If possible I need to be more explicit with referencing the current working directory. Will try and find a windows machine to test this on. |
its the same error as before |
Re: Windows -- if all else fails, |
We won't need |
|
@nuageklow I think we need to see permissions for the files in the repo you're using to bring up the app with I would try partitioning another drive on your computer, and then assign to to Docker for Windows, see what the result is. Depending, then we can go about working on file permissions. |
@bengineerdavis cb is the repo that contains docker-compose.yaml. also please ignore the docker-compose_ori.yaml. its just a copy of the original yaml file as i was trying to different things for to get the db container created. |
i've figured out the solution! the root of the issue for windows env is directory of the volume like everyone is indicating. apparently all we need to do is to change the the volume command from also windows requires a longer (detailed) command to call out a program, for example if i wanna pull psql i'll need to type: oh also since can anyone please tell me how to run i really hope that solves everything on the window's side lol thank you everyone for bearing with me and testing out the windows env. |
@billglover confirmed to work on Fedora 30 (Linux) OS with your setup using Some error messages and adjustments observed:
|
Did not mean to close this issue! |
@nuageklow Can you try |
This looks like a network timeout on docker registry's side. It should fix itself.
This is kind of expected because the manage container is not exactly a log running service. If this is somewhat confusing, then we can write a Makefile command to start only the services needed. We can also create Make commands for common operations like running migrations or getting into the django shell so that people don't have to remember the long docker-compose commands. eg: |
docker-compose.yaml
Outdated
volumes: | ||
db_data: {} | ||
nginx: {} | ||
cbv3_django_prototype: {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@billglover Just wanted to point out that we have all these named volumes defined but we're not really using these. Changing ./db_data
to just db_data
on line 19, ./nginx
to nginx
on line 55 and so on will make sure we use named volumnes instead of bind mounts. This should fix our problems on Windows too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sunu Are you saying that if @nuageklow makes only those changes in the docker-compose.yaml
file locally, it would probably work on her machine?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lpatmo i actually posted the same solution lastnight earlier on this same thread..i guess i worded it in a different way..?
#44 (comment)
but yes it worked and everything was installed properly.
for docker-compose run --rm manage shell
command it didnt work. i'll find a way and post it if i have a solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh sorry, I completely forgot about your comment from yesterday! Too many posts 😅
So the remaining items to do are:
1/ Update the docker file in this PR
2/ Figure out the shell command
3/ sunu's makefile
idea in a separate PR (maybe)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slight nuance... making the change and removing the ./
to use volume mounts means volumes cannot be mounted from within the project folder and so code doesn't get mapped through to the running application. This means development on the host isn't reflected in the running application so won't work for us.
I'm looking through documentation now to see if we have to drop back to bind mounts to achieve the set-up we are working towards.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sunu @billglover do you think it would be something windows specific only? i was sort of worried abt things might not be placed properly even thought it seemed like it ran smoothly.
also any expected end result be included would be ideal (e.g. how to define the volume is installed properly) so that a newbie like me would know how everything should look like if everything is mounted properly.
update: please disregard this reply i was writing this before bill's new comment #44 (comment). im gonna try things out again tonight / tomorrow. please let me know if u've found a new solution as well @sunu i can try it on my windows env :).
Working on an update today.
|
This post goes some way to explaining the behaviour we are seeing here: Define named volume with host mount in the docker compose file. Unfortunately, I think we need to make bind mounts work with Windows. |
@@ -27,3 +27,4 @@ django-debug-toolbar==2.0 # https://github.com/jazzband/django-debug-toolbar | |||
django-extensions==2.2.1 # https://github.com/django-extensions/django-extensions | |||
django-coverage-plugin==1.6.0 # https://github.com/nedbat/django_coverage_plugin | |||
pytest-django==3.5.1 # https://github.com/pytest-dev/pytest-django | |||
django-taggit==1.2.0 # https://github.com/jazzband/django-taggit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this package used for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sebbel django-taggit
is a tag management library that makes creating a global tagging system easier. Docs here: https://django-taggit.readthedocs.io/en/latest/
so props to @billglover. I could instantly contribute to the repository with the help of this docker setup. kudos! |
Switch to using volume mounts for the PostgreSQL data to allow docker-compose to be used on Windows 10 Pro.
Confirmed working on Windows 10 Pro. I have been able to complete the following:
I propose we run the fix for #53 once this is merged. If anyone would like a walk through of this running on Windows or Mac OS, please message me directly on Slack. |
Thank you so much for your work on this, @billglover! 🙏 And also to everyone who helped test this out! How do you feel about merging this, and we could file separate issues for: Then: |
I think this is ready to merge. I would suggest leaving Windows 10 Home until we have a need for it, but a Makefile and/or script for Windows users would be well worth it. Agree with 1, 2, 3. |
Love how well-documented everything is @billglover! |
The only remaining nitpick I have is to change restart policy of the containers from |
@sunu This isn’t a nitpick. This is a really good shout. I pulled this together on a desktop that I predominately access remotely. Completely overlooked the fact that the majority will be working on laptops. Will get that changed. |
Implement #41 by:
docker-compose up
as a way of instantiating a local development environmentA couple of known issues:
I'm creating this pull request for discussion. I encourage developers to clone my fork and experiment with the development experience so that we can get some feedback before this is merged.