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

Changed docker config and readme file #607

Merged
merged 5 commits into from Jan 22, 2020
Merged

Conversation

glagius
Copy link
Collaborator

@glagius glagius commented Jan 21, 2020

Minor changes have been made to simplify the installation of the project.

@codecov-io
Copy link

codecov-io commented Jan 21, 2020

Codecov Report

Merging #607 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #607   +/-   ##
=======================================
  Coverage   79.25%   79.25%           
=======================================
  Files          75       75           
  Lines        1523     1523           
=======================================
  Hits         1207     1207           
  Misses        316      316

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba07d0c...ec88c1f. Read the comment docs.

@@ -23,8 +23,6 @@ services:

db:
image: postgres:10
ports:
- "5432:${CODEBATTLE_DB_PORT}"
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

Я пробовала удалять, и локально всё запустилось, или оно на что-то другое может влиять?

Copy link
Contributor

@imamatory imamatory Jan 21, 2020

Choose a reason for hiding this comment

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

Просто не всегда удобно запускать в докере сервер, но по-умолчанию рекомендуется запускать все в докере.
Тем не менее хочется сохранить все способы запуска

@@ -5,7 +5,6 @@ vault_codebattle_github_secret: "eaac6e0cb603e652a133bde58902507a13cb0901"
vault_codebattle_db_hostname: "db"
vault_codebattle_db_username: "postgres"
vault_codebattle_db_password: ""
vault_codebattle_db_port: 5432
Copy link
Contributor

Choose a reason for hiding this comment

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

Переменную просто так удалять нельзя

@@ -20,7 +20,7 @@
- Clone repo

```bash
$ git clone https://github.com/hexlet-codebattle/codebattle.git
$ git clone git@github.com:hexlet-codebattle/codebattle.git
$ cd codebattle
$ mkdir -p tmp
$ echo 'asdf' > tmp/ansible-vault-password
Copy link
Contributor

Choose a reason for hiding this comment

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

Можно сократить шаги, добавив в development playbook шаг по добавлению "asdf" в tmp/ansible-vault-password

Copy link
Contributor

Choose a reason for hiding this comment

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

а, этого не нужно делать.

Copy link
Contributor

Choose a reason for hiding this comment

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

достаточно удалить строку
https://github.com/hexlet-codebattle/codebattle/blob/master/ansible.cfg#L2

@@ -5,7 +5,7 @@ vault_codebattle_github_secret: "eaac6e0cb603e652a133bde58902507a13cb0901"
vault_codebattle_db_hostname: "db"
vault_codebattle_db_username: "postgres"
vault_codebattle_db_password: ""
vault_codebattle_db_port: 5432
vault_codebattle_db_port: 5445
Copy link
Contributor

Choose a reason for hiding this comment

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

Вообще при разработке в докере эта переменная живет внутри контейнеров и ее значение никак не влияет на запуск. Достаточно той, которую экспоузит docker-compose

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

В таком случае какая разница, какой тут указан порт?

Copy link
Contributor

Choose a reason for hiding this comment

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

Это не так важно, но лучше дефолтный, потому что кастомный при отладке может сбить с толку

@imamatory imamatory mentioned this pull request Jan 21, 2020
@imamatory imamatory merged commit dfe4a3c into master Jan 22, 2020
@imamatory imamatory deleted the project-installation-changes branch January 22, 2020 16:55
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.

None yet

4 participants