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

New docker setup for emission-server #618

Merged
merged 9 commits into from
Dec 21, 2018

Conversation

alvinalexander
Copy link
Contributor

No description provided.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@@ -0,0 +1,31 @@
# python 3
FROM continuumio/anaconda3
Copy link
Contributor

Choose a reason for hiding this comment

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

This file doesn't include the changes to the Dockerfile in
#594

This is probably because you split the Dockerfile into two so the changes to Dockerfile were not merged into Dockerfile-base. Please merge the changes manually.

Also, as I pointed out in #617 (comment), after the change to miniconda, the server image is only 2GB. So I would recommend keeping one image for consistency but I will not oppose two images.

Copy link
Contributor

@shankari shankari left a comment

Choose a reason for hiding this comment

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

Much better. Just a few small changes and I think this is ready to merge.

docker/Dockerfile Outdated Show resolved Hide resolved
docker/Dockerfile Outdated Show resolved Hide resolved
docker/Dockerfile Show resolved Hide resolved
restart_policy:
condition: on-failure
ports:
- "27017:27017"
Copy link
Contributor

Choose a reason for hiding this comment

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

this is only if you want to access the DB from the host. Add a note to that effect?

docker/docker-compose.yml Outdated Show resolved Hide resolved
docker/start_script.sh Show resolved Hide resolved
Copy link
Contributor

@shankari shankari left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@shankari shankari left a comment

Choose a reason for hiding this comment

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

wait, not all the changes are done. Although the other changes are pretty small.

docker/Dockerfile Outdated Show resolved Hide resolved
docker/Dockerfile Outdated Show resolved Hide resolved
@shankari shankari merged commit e76681b into e-mission:master Dec 21, 2018
jf87 pushed a commit to jf87/e-mission-server that referenced this pull request Jun 21, 2021
* Resolve conflict in start_script.sh

* Resolve conflict in Dockerfile

* Add composefile

* Update docker README.md and Dockerfile

* Update README

* Moved everything into Dockerfile and deleted Dockerfile-base

* Add Maintainer

* Mount volume to store db data by default

* Pull from emssion-server repo and install Vim
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants