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

consider ENV for init.sh #399

Merged
merged 3 commits into from
Sep 1, 2019
Merged

consider ENV for init.sh #399

merged 3 commits into from
Sep 1, 2019

Conversation

walf443
Copy link
Collaborator

@walf443 walf443 commented Aug 31, 2019

@walf443 walf443 self-assigned this Aug 31, 2019
@walf443 walf443 force-pushed the fix/consider_env_on_init.sh branch from 0de064d to 58eaf98 Compare August 31, 2019 16:47
@walf443
Copy link
Collaborator Author

walf443 commented Aug 31, 2019

うーん。perl ではENV名がgolangと違うな... https://github.com/catatsuy/isucon9-qualify/blob/master/webapp/perl/lib/Isucari/Web.pm#L111

@walf443
Copy link
Collaborator Author

walf443 commented Aug 31, 2019

@walf443
Copy link
Collaborator Author

walf443 commented Aug 31, 2019

@walf443
Copy link
Collaborator Author

walf443 commented Aug 31, 2019

rubyもgoと同じ catatsuy@52eea76#diff-2bcdf1d251ca496ad7c306e5a9338835R22

node.js実装はperlと同じになっているが既に手元では変更した

@walf443
Copy link
Collaborator Author

walf443 commented Aug 31, 2019

このPRではperlだけ別っぽいので、他実装にあわせておけばよさそうかなー

@walf443
Copy link
Collaborator Author

walf443 commented Aug 31, 2019

app側のenv名と https://github.com/catatsuy/isucon9-qualify/blob/master/provisioning/roles/bootstrap/files/home/isucon/env.sh#L1 が一致してないけどこれは問題ないかな?

@walf443
Copy link
Collaborator Author

walf443 commented Aug 31, 2019

phpはgoと同じくMYSQL_XXXを見るように実装はなっているが、

https://github.com/catatsuy/isucon9-qualify/blob/master/provisioning/roles/php/files/isucari.php-fpm.conf#L17 ではDB_XXX をセットするようになっているらしい

@walf443
Copy link
Collaborator Author

walf443 commented Aug 31, 2019

perlではprovisioning側にENV設定がないようなので他の実装へあわせておきました。

@walf443 walf443 requested a review from kazeburo August 31, 2019 17:58
export MYSQL_PORT=${MYSQL_PORT:-3306}
export MYSQL_USER=${MYSQL_USER:-isucari}
export MYSQL_DBNAME=${MYSQL_DBNAME:-isucari}
export MYSQL_PASS=${MYSQL_PASS:-isucari}
export LANG="C.UTF-8"
Copy link
Collaborator

Choose a reason for hiding this comment

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

MYSQL_PWDにして、-p オプション使わないようにしたいかな
動かないとかあれば、こちらでよいです

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

-pオプション使わないようにした場合、/initializeなどで実行したときにパスワードの入力はどうやらせるのでしょうか?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

この問題は、 https://github.com/catatsuy/isucon9-qualify/blob/master/webapp/go/main.go#L463 でこのシェルスクリプトを実行するようになっているが、 https://github.com/catatsuy/isucon9-qualify/pull/316 でnodejsの場合、MySQL 5.7を使わないといけないため、手元でうまく動かすためには設定変更した状態で動くようにしないといけない(手元でMySQL 5.8などをinstallしていれば...、という文脈でやり始めました。

Copy link
Collaborator

Choose a reason for hiding this comment

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

MYSQL_PWD環境変数が、パスワードとして使われます〜

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

なるほど、そういう仕組み用の環境変数があるんですねー

@kazeburo
Copy link
Collaborator

kazeburo commented Sep 1, 2019

あざますー
OKですー

@walf443 walf443 merged commit db57235 into master Sep 1, 2019
@walf443 walf443 deleted the fix/consider_env_on_init.sh branch September 1, 2019 00:42
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.

2 participants