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

Docker dev environment #103

Merged
merged 9 commits into from Jan 11, 2017
Merged

Docker dev environment #103

merged 9 commits into from Jan 11, 2017

Conversation

tamtamchik
Copy link
Member

@tamtamchik tamtamchik commented Jan 3, 2017

Docker development setup

Run / Stop (daemon mode)

$ docker-compose up -d
$ docker-compose stop

Forwarded ports and access on host machine

Web: http://localhost:3000
Database: postgres://postgres@localhost:6543

Some helpers

Action Command
Bundler docker-compose exec app bundle install
Setup DB docker-compose exec app bundle exec rails db:create db:migrate
Console docker-compose exec app bundle exec rails c

@tamtamchik tamtamchik self-assigned this Jan 6, 2017
@vtambourine
Copy link
Member

Я бы делал либо run --rm, либо exec.

@tamtamchik
Copy link
Member Author

@vtambourine да, exec оно правильнее

@tamtamchik
Copy link
Member Author

tamtamchik commented Jan 6, 2017

Хотел настроить https://circleci.com для работы с docker-compose но сходу не завелось. Это на случай если увидите [Build] Failed в почте :)

@tamtamchik tamtamchik changed the title WIP: Docker dev environment Docker dev environment Jan 6, 2017
db:
image: postgres
ports:
- "6543:5432"
Copy link
Member

Choose a reason for hiding this comment

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

А зачем пробрасывать порт из контейнера бд на хост?

Copy link
Member Author

Choose a reason for hiding this comment

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

Мало ли надо будет в базу залезть, посмотреть что там...

Copy link
Member

Choose a reason for hiding this comment

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

Я тоже использую такую настройку, чтобы смотреть, что с базой творится.

@@ -1,6 +1,8 @@
development: &default
adapter: postgresql
database: it61_development
host: <%= ENV.fetch("DATABASE_HOST", "localhost") %>
Copy link
Member

Choose a reason for hiding this comment

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

Может оставить везде единообразно и каждый сам укажет в .env нужные ему реквизиты?

Copy link
Member Author

@tamtamchik tamtamchik Jan 9, 2017

Choose a reason for hiding this comment

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

В смысле сделать такой же DATABASE_URL как и в настрйоке на прод?

Copy link
Member

Choose a reason for hiding this comment

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

Ага. Я бы вообще использовал database_url, чтобы все параметры подключения вынести в env, а для тестового окружения: <%= ENV.fetch("DATABASE_URL") %>_test.

Copy link
Member Author

Choose a reason for hiding this comment

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

@kovalevsky согласен.

Copy link
Member Author

Choose a reason for hiding this comment

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

@vitallium ^^ тыц. Сделай просто

DATABASE_URL = postgres://root@localhost:5432/it61

@@ -16,4 +18,4 @@ production:
min_messages: warning
pool: <%= [Integer(ENV.fetch("MAX_THREADS", 5)), Integer(ENV.fetch("DB_POOL", 5))].max %>
timeout: 5000
url: <%= ENV.fetch("DATABASE_URL", "") %>
url: <%= ENV.fetch("DATABASE_URL", "") %>
Copy link
Member

Choose a reason for hiding this comment

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

Для чего указывать пустую строку по умолчанию, путь лучше райзится исключение, если с конфигурацией что-то не так.

Copy link
Member Author

Choose a reason for hiding this comment

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

См. комент выше, я просто лишний пробел убрал :)

@@ -16,4 +18,4 @@ production:
min_messages: warning
pool: <%= [Integer(ENV.fetch("MAX_THREADS", 5)), Integer(ENV.fetch("DB_POOL", 5))].max %>
Copy link
Member

Choose a reason for hiding this comment

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

Может достаточно одной переменной?

Copy link
Member Author

@tamtamchik tamtamchik Jan 9, 2017

Choose a reason for hiding this comment

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

Тут ничего не менял, подумал раз так есть, может так и надо...

Copy link
Member

Choose a reason for hiding this comment

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

@vitallium, это нужно?)

Copy link
Member

Choose a reason for hiding this comment

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

Нужно. прод работает в контейнерах, и это нужно для горизонтального масштабирования.

Copy link
Member

Choose a reason for hiding this comment

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

Это понятно, но почему мы не можем использовать всегда какую-то одну из этих env переменных?

Copy link
Member

Choose a reason for hiding this comment

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

А по поводу потоков, еще сам schneems писал, что:

You need your pool to at least be as large as your puma thread count.

Copy link
Member Author

Choose a reason for hiding this comment

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

Флудеры! 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Да, давайте (если есть желание) это обсудим в #rndrug :-) а то никогда не смержим

Copy link
Member

Choose a reason for hiding this comment

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

И тут я такой заметил, что в puma.rb переменная называется RAILS_MAX_THREADS, а не MAX_THREADS :O

Copy link
Member

Choose a reason for hiding this comment

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

Ну т.е. это MAX_THREADS – это что-то неведомое. Я бы это выкосил

@@ -0,0 +1,7 @@
.git*
db/*.sqlite3
db/*.sqlite3-journal
Copy link
Member

Choose a reason for hiding this comment

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

sqlite вроде в проекте нет и вряд ли будет)

Copy link
Member Author

Choose a reason for hiding this comment

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

Мало-ли кто-то будет баловаться :) Стандартная затычка

Copy link
Member

Choose a reason for hiding this comment

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

Я бы это убрал. Когда кто-то будет баловать с sqlite, он добавит игнор-файлы. А до тех пор это вводит в заблуждение.

Copy link
Member Author

Choose a reason for hiding this comment

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

Убрал от греха подальше.

Copy link
Member

Choose a reason for hiding this comment

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

Когда кто-то будет баловать с sqlite, он добавит игнор-файлы. А до тех пор это вводит в заблуждение.

Я, конечно, опаздал, но мое мнение такое:
+1, нужно же понимать, зачем нужен докеригнор. Если кто-то будет баловаться и соберет контейнер со всяким мусором, то это его проблемы (хотя, как вообще можно баловаться с sqlite, на проекте, который с ним не заработает).
Мы игнорим в контейнерах некоторые файлы, потому что хотим обеспечить минимальный размер образа и изменившихся слоев. Это полезно для диска разработчика и критично для деплоя в прод.
Если разработчик соберет образы и контейнеры с хламом, то это его проблемы и всегда можно сделать ишью.

ps. В общем, я всегда сильно против того, что мотивировано словами "может пригодиться, но мы не знаем когда и кому".

@tamtamchik
Copy link
Member Author

@vitallium @kovalevsky вы пробовали поднимать у себя, все работает as expected? Я так понимаю если нет других проблем кроме переменных для прода, то можно мержить.

@tamtamchik
Copy link
Member Author

🥇 Самый обсуждаемый мерж! 🥇

Copy link
Member

@kovalevsky kovalevsky left a comment

Choose a reason for hiding this comment

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

Непосредственно по pr вопросов нет, все работает как надо

Copy link
Member

@vitallium vitallium left a comment

Choose a reason for hiding this comment

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

LGTM

@tamtamchik tamtamchik merged commit 3b3d7f6 into IT61:master Jan 11, 2017
@tamtamchik tamtamchik deleted the docker-dev-setup branch January 11, 2017 15:03
@dtulyakov
Copy link

надо бы в редми поправить
docker-compose start запускает контейнеры в фореграунд режиме
docker-up нужен для того что бы смотреть вывод на экран, аналогично docker-compose logs ....

@tamtamchik
Copy link
Member Author

@dtulyakov насколько я знаю, - start не создает контейнеры, т.е. нужно будет еще вызывать create, а up -d как раз это же и делает, то не вижу смысла менять. Поправьте меня если не прав.

@dtulyakov
Copy link

@tamtamchik да проверил менять ридми смысла нет т.к., действительно up создаёт контейнер если его нет
а я create юзаю ))

@@ -1,6 +1,5 @@
development: &default
adapter: postgresql
Copy link
Member

Choose a reason for hiding this comment

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

Вот эта строчка сломала локальное окружение (╯°□°)╯︵ ┻━┻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants