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

data, outputディレクトリをコンテナイメージ内に入れるように変更 #269

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Flora-alter
Copy link

元々ボリュームで後挿しにしていたdataディレクトリとoutputディレクトリについて、DockerfileでCOPYすることでコンテナイメージ内に入れるように変更

@saza-ku
Copy link
Contributor

saza-ku commented Oct 27, 2023

ありがとうございます!
ぱっと見良さそうです
動作確認は済んでますか?

@Flora-alter
Copy link
Author

チェックありがとうございます
そういえば動作確認忘れてました
自分の環境でAdventリポジトリをDocker composeできればいいって感じですかね

@saza-ku
Copy link
Contributor

saza-ku commented Oct 27, 2023

そうですね、おそらくそれで動くと思います
ただ開発環境用の docker-compose.yml も書き換える必要がありますね:eyes:

@Flora-alter
Copy link
Author

docker-compose.ymlのservices:advent:imageのところを変えればいい感じですか?
あるいはポート番号がすでに使われているから変えないといけないとかですかね?

@Flora-alter
Copy link
Author

手元のUbuntuではbuildとrunはできました。

@saza-ku
Copy link
Contributor

saza-ku commented Oct 29, 2023

あー、なるほど、開発環境用の docker-compose では手元のファイル全部ボリュームに入れてるのか

@saza-ku
Copy link
Contributor

saza-ku commented Oct 29, 2023

すみません、これ最終的に全部のファイルを COPY してきてるので新たに data と output を COPY する必要はなかったですね...

https://github.com/camphor-/advent/blob/master/Dockerfile#L11

@saza-ku
Copy link
Contributor

saza-ku commented Oct 29, 2023

つまり直すべきは docker-compose.prod.yml のみですが、これはもう使わないファイルなので、これを消す修正だけお願いします:pray:

Dockerfile の修正は必要なかったです:bow::bow:

@Flora-alter
Copy link
Author

アスタリスクついてないのに全部コピーするんですね...
Dockerfileガチ初心者ゆえに気付きませんでした
ではその通りに直しておきます

・dockerfile already copies all subdirectories
@Flora-alter
Copy link
Author

下記の変更を加えました
・docker-compose.ymlの削除
・dockerfileにおけるdata, outputディレクトリのコピー記述を削除

@saza-ku
Copy link
Contributor

saza-ku commented Oct 29, 2023

あ、ごめんなさい!消すのは docker-compose.prod.yml のみでした:pray:

で、docker-compose.yml については、ボリュームの設定を消しておきましょうか

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