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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docker dev setup #2849

Merged
merged 8 commits into from Mar 15, 2018

Conversation

@deivid-rodriguez
Copy link
Contributor

commented Feb 27, 2018

馃帺 What? Why?

This PR catches up with decidim/docker#20, but I also take the chance to propose the development setup I've been intermitently using for a while. The idea, as I mentioned in #2755, is to provide docker binstubs for the most usual tools we use for development, and that any instructions for working on a regular development environment can be used "in docker" by replacing the main command with the docker binstub.

馃搶 Related Issues

馃搵 Subtasks

None.

馃摲 Screenshots (optional)

None.

@ghost ghost added the in-progress label Feb 27, 2018
@@ -0,0 +1,8 @@
#!/bin/bash

gem=${1%%/*}

This comment has been minimized.

Copy link
@mrcasals

mrcasals Feb 28, 2018

Contributor

How are you supposed to use this command?

#!/bin/bash

gem=${1%%/*}
spec=${1#*/}

This comment has been minimized.

Copy link
@mrcasals

mrcasals Feb 28, 2018

Contributor

This is some bash black magic, what does this do?

I'm not asking for docs, but I'm curious 馃ぃ

This comment has been minimized.

Copy link
@deivid-rodriguez

deivid-rodriguez Feb 28, 2018

Author Contributor

I forgot I had added this! It allows you to run any spec from the root folder, for example, d/rspec decidim-core/spec/system/my_system_spec.rb

This comment has been minimized.

Copy link
@mrcasals

mrcasals Feb 28, 2018

Contributor

Oh, nice! Too bad we can't reuse this outside docker, because it's super useful!

This comment has been minimized.

Copy link
@deivid-rodriguez

deivid-rodriguez Feb 28, 2018

Author Contributor

We can, if we add regular binstubs xD. It'd be a matter of doing the same thing in ruby inside the binstub.

This comment has been minimized.

Copy link
@mrcasals

mrcasals Feb 28, 2018

Contributor

I might be missing something, but if we add the equivalent for a non-Docker environment then we need to keep changing the folder to run the tests 馃

Unless we modify the main spec_helper so that it knows how to run the tests from the top-level folder... Anyway, that's another discussion 馃槃

This comment has been minimized.

Copy link
@deivid-rodriguez

deivid-rodriguez Feb 28, 2018

Author Contributor

Yes, I mean "keep changing the folder when running the tests". Switch folder, run test(s), switch back.

This comment has been minimized.

Copy link
@mrcasals

mrcasals Feb 28, 2018

Contributor

Ah, I see 馃憣

@mrcasals

This comment has been minimized.

Copy link
Contributor

commented Feb 28, 2018

@josepjaume can you review this please?

@mrcasals mrcasals requested a review from josepjaume Feb 28, 2018
@@ -0,0 +1,3 @@
#!/bin/bash

docker-compose run --rm decidim bash "$@"

This comment has been minimized.

Copy link
@josepjaume

josepjaume Feb 28, 2018

Contributor

Not sure docker-compose run is the best way to do this. I usually just launch the app via docker-compose up -d (this starts postgresql & potentially any other dependency) and then execute the commands via docker-compose exec decidim whatever_command. This boots faster and is more efficient memory-wise.

This comment has been minimized.

Copy link
@deivid-rodriguez

deivid-rodriguez Feb 28, 2018

Author Contributor

The thing is... the decidim repo is not an app, so I'm not sure if docker-compose up means anything here... According to the current docker-compose.yml file, it would mean to generate a decidim application via the decidim executable (since it has decidim configured as the command for the decidim service). But I'm not really using it this way.

To boot a development application, I use the rails binstub added here. For the rest of the commands, I use the same command as in normal development, for example, d/rake test_app to generate a test application, d/rake development_app to generate a development application, d/rake test_all to run all tests, d/bundle install to install dependencies, and so on.

Also, I'm not sure about the speed and memory efficiency argument since I haven't noticed any problem with it, it works more or less like regular development for me. So... I don't know, maybe there are better ways to do it, I haven't researched since it hasn't bothered me.

Finally, I don't really use this specific binstub a lot except for troubleshooting things inside the image. But I thought it could be provided for completeness so the user is able to easily run arbitrary commands inside the image.

This comment has been minimized.

Copy link
@josepjaume

josepjaume Mar 5, 2018

Contributor

@deivid-rodriguez but you'll need postgres and other dependencies running when you generate a new app or test, right? Using docker run whatever will start postgres and other dependencies every time you run the command - by using docker exec you execute commands inside existing containers. That's why I say you'll cut boot time and you'll save memory :). Or maybe I'm missing something?

This comment has been minimized.

Copy link
@deivid-rodriguez

deivid-rodriguez Mar 5, 2018

Author Contributor

@josepjaume I understand what you're saying: we don't need to start postgreSQL everytime you run a docker command, you can do that upfront. What I'm saying is that it doesn't matter to me to start postgreSQL every time since I haven't noticed the degradation, and making it work the way you suggest needs work. So, I'm totally up for doing it the way you like, but I think this is a good start since it works.

However, I'm doing some tests and I'm actually not sure about what you're saying... It seems to me that postgreSQL is booted just once, or at least there's some kind of caching going on that makes it fast after the first time...

$ docker ps
CONTAINER ID        IMAGE               COMMAND                  CREATED             STATUS              PORTS               NAMES
06dc890c4cff        redis               "docker-entrypoint.s鈥"   2 weeks ago         Up About a minute   6379/tcp            decidim_redis_1
6b745e5319a7        postgres            "docker-entrypoint.s鈥"   2 weeks ago         Up About a minute   5432/tcp            decidim_pg_1

$ docker stop 06dc890c4cff 6b745e5319a7
06dc890c4cff
6b745e5319a7

$ docker ps
CONTAINER ID        IMAGE               COMMAND             CREATED             STATUS              PORTS               NAMES

$ time docker-compose run --rm decidim ls
Starting decidim_pg_1 ... 
Starting decidim_pg_1
Starting decidim_redis_1 ... 
Starting decidim_redis_1 ... done
+ stat -c %u /code/Gemfile
+ USER_UID=1000
+ stat -c %g /code/Gemfile
+ USER_GID=1000
+ export USER_UID
+ export USER_GID
+ usermod -u 1000 decidim
+ groupmod -g 1000 decidim
+ usermod -g 1000 decidim
+ chown -R -h 1000 /usr/local/bundle
+ chgrp -R -h 1000 /usr/local/bundle
+ /usr/bin/sudo -EH -u decidim ls
CHANGELOG.md	    README.md	  d			  decidim-core			   decidim-surveys	  docs		 spec		      yarn.lock
CODE_OF_CONDUCT.md  Rakefile	  decidim-accountability  decidim-debates		   decidim-system	  jsconfig.json  tsconfig.json
CONTRIBUTING.md     bin		  decidim-admin		  decidim-dev			   decidim-verifications  lib		 tslint.json
Dockerfile.design   codecov.yml   decidim-api		  decidim-meetings		   decidim.gemspec	  logo.svg	 webpack.config.js
Gemfile		    config	  decidim-assemblies	  decidim-pages			   decidim_app-design	  node_modules	 webpack.d.ts
Gemfile.lock	    coverage	  decidim-budgets	  decidim-participatory_processes  development_app	  package.json	 webpack.report.html
LICENSE-AGPLv3.txt  crowdin.yaml  decidim-comments	  decidim-proposals		   docker-compose.yml	  pkg		 yarn-error.log

real	0m4.263s
user	0m0.956s
sys	0m0.112s

$ docker ps
CONTAINER ID        IMAGE               COMMAND                  CREATED             STATUS              PORTS               NAMES
06dc890c4cff        redis               "docker-entrypoint.s鈥"   2 weeks ago         Up 6 seconds        6379/tcp            decidim_redis_1
6b745e5319a7        postgres            "docker-entrypoint.s鈥"   2 weeks ago         Up 7 seconds        5432/tcp            decidim_pg_1

$ time docker-compose run --rm decidim ls
Starting decidim_redis_1 ... done
Starting decidim_pg_1 ... done
+ stat -c %u /code/Gemfile
+ USER_UID=1000
+ stat -c %g /code/Gemfile
+ USER_GID=1000
+ export USER_UID
+ export USER_GID
+ usermod -u 1000 decidim
+ groupmod -g 1000 decidim
+ usermod -g 1000 decidim
+ chown -R -h 1000 /usr/local/bundle
+ chgrp -R -h 1000 /usr/local/bundle
+ /usr/bin/sudo -EH -u decidim ls
CHANGELOG.md	    README.md	  d			  decidim-core			   decidim-surveys	  docs		 spec		      yarn.lock
CODE_OF_CONDUCT.md  Rakefile	  decidim-accountability  decidim-debates		   decidim-system	  jsconfig.json  tsconfig.json
CONTRIBUTING.md     bin		  decidim-admin		  decidim-dev			   decidim-verifications  lib		 tslint.json
Dockerfile.design   codecov.yml   decidim-api		  decidim-meetings		   decidim.gemspec	  logo.svg	 webpack.config.js
Gemfile		    config	  decidim-assemblies	  decidim-pages			   decidim_app-design	  node_modules	 webpack.d.ts
Gemfile.lock	    coverage	  decidim-budgets	  decidim-participatory_processes  development_app	  package.json	 webpack.report.html
LICENSE-AGPLv3.txt  crowdin.yaml  decidim-comments	  decidim-proposals		   docker-compose.yml	  pkg		 yarn-error.log

real	0m2.506s
user	0m0.808s
sys	0m0.100s

This comment has been minimized.

Copy link
@josepjaume

josepjaume Mar 15, 2018

Contributor

Oh! That's weird. Maybe I don't know enough about docker... Let's merge this and iterate over it, if we need to. Thanks!

This comment has been minimized.

Copy link
@deivid-rodriguez

deivid-rodriguez Mar 15, 2018

Author Contributor

Thanks @josepjaume, we'll learn more as we go!

@@ -98,7 +98,11 @@ plugins:
enabled: true

exclude_patterns:
- docs/*

This comment has been minimized.

Copy link
@andreslucena

andreslucena Mar 2, 2018

Member

Wouldn't be better to leave this? so we can add new files on docs/ and don't have to update this file every time. Also, why we only need getting-started.md ?

This comment has been minimized.

Copy link
@deivid-rodriguez

deivid-rodriguez Mar 2, 2018

Author Contributor

You won't have to. We'll only remove entries from here, not add more. When you add a new file, it will be analyzed because it's not excluded here, it will be linted, issues will be fixed, and good to go!

These exclusions are there because those files don't currently comply to markdown style rules, so they're excluded from the markdownlint check. In this PR I linted one of them so I removed the exclusion (by listing all files except for the one that I linted).

@deivid-rodriguez deivid-rodriguez force-pushed the docker_dev_setup branch from 6649beb to 1d01cff Mar 5, 2018
@codecov

This comment has been minimized.

Copy link

commented Mar 5, 2018

Codecov Report

Merging #2849 into master will increase coverage by 4.42%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2849      +/-   ##
==========================================
+ Coverage    94.3%   98.73%   +4.42%     
==========================================
  Files        1662     1670       +8     
  Lines       39590    39730     +140     
==========================================
+ Hits        37334    39226    +1892     
+ Misses       2256      504    -1752
@deivid-rodriguez deivid-rodriguez force-pushed the docker_dev_setup branch from 1d01cff to bf87fea Mar 8, 2018
@mrcasals

This comment has been minimized.

Copy link
Contributor

commented Mar 13, 2018

@deivid-rodriguez @josepjaume what's the status on this? Should we merge it?

@deivid-rodriguez deivid-rodriguez force-pushed the docker_dev_setup branch from bf87fea to a49ba95 Mar 14, 2018
@deivid-rodriguez

This comment has been minimized.

Copy link
Contributor Author

commented Mar 14, 2018

For what it's worth, I just rebased it :)

@josepjaume josepjaume merged commit 15ca59d into master Mar 15, 2018
21 checks passed
21 checks passed
ci/circleci: accountability Your tests passed on CircleCI!
Details
ci/circleci: admin Your tests passed on CircleCI!
Details
ci/circleci: api Your tests passed on CircleCI!
Details
ci/circleci: assemblies Your tests passed on CircleCI!
Details
ci/circleci: budgets Your tests passed on CircleCI!
Details
ci/circleci: build_design_app Your tests passed on CircleCI!
Details
ci/circleci: build_test_app Your tests passed on CircleCI!
Details
ci/circleci: comments Your tests passed on CircleCI!
Details
ci/circleci: core Your tests passed on CircleCI!
Details
ci/circleci: debates Your tests passed on CircleCI!
Details
ci/circleci: main Your tests passed on CircleCI!
Details
ci/circleci: meetings Your tests passed on CircleCI!
Details
ci/circleci: pages Your tests passed on CircleCI!
Details
ci/circleci: processes Your tests passed on CircleCI!
Details
ci/circleci: proposals Your tests passed on CircleCI!
Details
ci/circleci: surveys Your tests passed on CircleCI!
Details
ci/circleci: system Your tests passed on CircleCI!
Details
ci/circleci: verifications Your tests passed on CircleCI!
Details
codeclimate All good!
Details
codecov/patch Coverage not affected when comparing d00e80e...a49ba95
Details
codecov/project 98.73% (+4.42%) compared to d00e80e
Details
@josepjaume josepjaume deleted the docker_dev_setup branch Mar 15, 2018
@ghost ghost removed the in-progress label Mar 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can鈥檛 perform that action at this time.