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

Add redis to Docker setup #155

Closed
abbycabs opened this issue Feb 25, 2016 · 13 comments
Closed

Add redis to Docker setup #155

abbycabs opened this issue Feb 25, 2016 · 13 comments
Labels

Comments

@abbycabs
Copy link
Member

Redis was added after we set the project up with Docker. Add redis and updates the docs!

@sudheesh001
Copy link
Contributor

@acabunoc It looks like this issue was resolved in a086d2a

Should the fix for this issue be from

You can use Docker to bring up a quick instance of the app to develop against. This way you dont need to have node or mongo installed on your host.

to

You can use Docker to bring up a quick instance of the app to develop against. This way you don't need to have node, mongo or redis installed on your host.

should I make changes to the dockerfile as follows ?

FROM node:0.12
FROM redis

RUN mkdir /src
WORKDIR /src

COPY . /src

# build the project only if it needs to
RUN if [ ! -d "node_modules" ]; then npm install; fi

CMD redis-server
CMD npm start

# Expose ports locally
EXPOSE 6379

@josmas
Copy link
Member

josmas commented Mar 8, 2016

I am a bit confused about the use of this Dockerfile and the docker-compose file.

If the aim is to quickly setup a development environment, then copying the sources into the container (as it is done in the file) is far from ideal, as they can only be edited from within the container. Sharing the sources from the host would be the easiest way to use docker for development.

If the aim is to quickly put together a few containers for deployment, then it is fine as it is, but it should be properly documented in the readme (stating the intent of the setup).

Unrelated to that, and with regards to the last comment; the redis server runs on its own container (follow the docker-compose file), so there's no need to make those changes to the file.

The issue, as it is framed, is already taken care of in the docker-compose file.

@abbycabs
Copy link
Member Author

abbycabs commented Mar 9, 2016

Wow, yes this already taken care of. By me. thanks @sudheesh001 & @josmas!

@josmas I'm not very familiar with Docker. @jonocodes helped us set this up and maybe he has more insight into the setup.

@jonocodes
Copy link
Contributor

Hello all.

Yes @sudheesh001, you should not need to edit the Dockerfile since compose it taking care of redis.

@josmas , I think the goal was to use docker to bring up an environment you could use quickly so you can try to use paperbadger with another app. Using Docker to develop paperbadger is a good idea, and is not really covered by this setup. But it would be easy to share in a directory like you said if that was use case. I'm happy to help with any of that.

As a side note, something to consider is distrubuting paperbadger via docker images. You would basically build the docker image that is already specified here and push it to docker hub. Then you could direct people who want to try (not develop) paperbadger to pull it from there. It would take a little maintaince but may be a way of doing some simple packaging/distribution. I'm seeing a lot of projects do that these days.

@josmas
Copy link
Member

josmas commented Mar 18, 2016

hey @jonocodes thanks for the info! It makes sense.

I have done something similar to what you mention in your last point with the CoderDojo sources, and it's been really useful at hackathons (instead of spending half the morning installing stuff, you can just spin a container in no time).

In any case, I don't think the current setup is being used a lot, so I guess there's room for changes. What do you all think about the following changes?:

  1. Build from node 4 (instead of 0.12; this may need some other changes)
  2. Share the sources with the host <-- this can be done by adding a run script so it can be configurable
  3. Remove npm start from the file and provide two scripts, one to simply run bash, and one to run npm start

@jonocodes
Copy link
Contributor

Build from node 4 (instead of 0.12; this may need some other changes)

I dont know much about node versions, but perhaps this should be updated then?
https://github.com/mozillascience/PaperBadger/blob/master/package.json#L52

Share the sources with the host <-- this can be done by adding a run script so it can be configurable

Sure. This makes sense as long as there is no plan to create distributable images.

Remove npm start from the file and provide two scripts, one to simply run bash, and one to run npm start

What would the bash one be for?

@josmas
Copy link
Member

josmas commented Mar 22, 2016

Hi @jonocodes

yep, package.json can be changed. There was a short convo on gitter about how 0.12 will go out of LTS by the end of the year. There's also the use of npm 2 and changes in npm 3 that would improve versions being pulled in.

With regards to the scripts, the first one would be to provide the existing functionality (just run the app), and the bash one would give more flexibility while developing. At times, I find that I want to restart the server (or run tests, or stuff like that), and the only way to restart it now is to bring down the whole container and start it up again.

@jonocodes
Copy link
Contributor

With regards to the scripts, the first one would be to provide the existing functionality (just run the app), and the bash one would give more flexibility while developing. At times, I find that I want to restart the server (or run tests, or stuff like that), and the only way to restart it now is to bring down the whole container and start it up again.

Perhaps the things you are trying to do would work by using 'docker exec' to shell into the container instead of creating a run variant.

@josmas
Copy link
Member

josmas commented Mar 22, 2016

It would work for some things with exec ... bash but not in the case of restarting the server.

@jonocodes
Copy link
Contributor

I would probably restart the container completely instead, but sure.

@jonocodes
Copy link
Contributor

Per your suggestion, I have redone the docker setup to mount local sources instead of building an image. There is no Dockerfile anymore - but requires a newer version of Docker. Since the project requirements are so small, I was able to put all the setup into the docker-compose file. It will also be easier for @josmas to extend to create a bash variation for his needs. See pr #179.

@abbycabs
Copy link
Member Author

Amazing, thanks @jonocodes! I'll take a closer look today

@josmas
Copy link
Member

josmas commented May 12, 2016

This was merged as #179 so closing now.

@josmas josmas closed this as completed May 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants