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 docker-compose #33

Merged
merged 5 commits into from
May 21, 2018
Merged

Add docker-compose #33

merged 5 commits into from
May 21, 2018

Conversation

dpordomingo
Copy link
Contributor

@dpordomingo dpordomingo commented May 16, 2018

If accepted it would be only needed to run:

Run latest released image:

$ GITBASEPG_REPOS_FOLDER=./repos docker-compose up

Run from sources (useful when developing or willing to run tip of master...)

$ GITBASEPG_REPOS_FOLDER=./repos make compose-serve

@dpordomingo dpordomingo added the enhancement New feature or request label May 16, 2018
@dpordomingo dpordomingo self-assigned this May 16, 2018
@dpordomingo dpordomingo requested review from carlosms and bzz May 16, 2018 13:49
image: "bblfsh/bblfshd"
privileged: true
volumes:
- /var/lib/bblfshd:/var/lib/bblfshd
Copy link
Contributor

Choose a reason for hiding this comment

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

It wouldn't work on macos. Please consider using named volume instead:
bblfsh/bblfshd#136

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@smacker I rebased and addressed your suggestion about named volumes; It was my first time;) did I do it properly?

@carlosms
Copy link
Contributor

Looks great!

Maybe this is asking too much, but would it be possible to build a bblfsh container that already has all the drivers installed? I imagine something like:

compose:

bblfsh:
  build:
    context: .
    dockerfile: Dockerfile-bblfsh-drivers

Dockerfile-bblfsh-drivers

FROM bblfsh/bblfshd
RUN docker-compose exec bblfsh bblfshctl driver install --recommended

@dpordomingo
Copy link
Contributor Author

@carlosms I included the driver installation process in the docker-compose.yml itself so they would be automatically installed if needed when app is ran (only for the first time).
Adding the drivers in the bblfshd image can be... weird.

Is it better for you?

Copy link
Contributor

@carlosms carlosms left a comment

Choose a reason for hiding this comment

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

This is great!

@bzz
Copy link
Contributor

bzz commented May 18, 2018

Is it supposed to be on top of the latest master?

screen shot 2018-05-18 at 9 58 36 am

Copy link
Contributor

@bzz bzz left a comment

Choose a reason for hiding this comment

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

tried on macOS - works like a charm. 👏

--env GITBASEPG_DB_CONNECTION="gitbase@tcp(gitbase:3306)/none?maxAllowedPacket=4194304"
--name gitbase_playground
srcd/gitbase-playground:latest
$ GITBASEPG_ENV=dev REPOS_FOLDER=./repos GITBASEPG_ENV=dev docker-compose up
Copy link
Contributor

@bzz bzz May 18, 2018

Choose a reason for hiding this comment

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

as this is a quickstart that first time users might follow - do you think it might be worth not to assume that they know, but rather state explicitly here that after this command, if a user does Ctrl+C - he will still have the containers running and have to stop/kill/rm those?

Copy link
Contributor

Choose a reason for hiding this comment

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

with this command, Ctrl-C would kill containers.

@dpordomingo
Copy link
Contributor Author

I updated the PR desc so:

# Run latest released image:
$ REPOS_FOLDER=./repos docker-compose up

# Run from sources (useful when developing or willing to run the tip of master...)
$ REPOS_FOLDER=./repos make compose-serve

@bzz This ☝️ address your #33 (comment) What it happened to you is that you started the latest tag (v0.0.1) not containing the frontend. To build the tip of master or whatever another commit, you should build and run using make compose-serve

@smacker @carlosms could you PTAL again? I included the two running options as agreed.

@eiso you might be interested in this b874407, that address your suggestions made by #36


You can locally build and deploy `gitbase-playground` and its dependencies using [`docker-compose`](https://docs.docker.com/compose/install/)

If you preffer to run `gitbase-playground` with [`docker-compose`](https://docs.docker.com/compose) (without taking care of the app dependencies), you can follow [the playground compose quickstart](quickstart.md)
Copy link
Contributor

Choose a reason for hiding this comment

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

preffer -> prefer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks!!!

@smacker
Copy link
Contributor

smacker commented May 18, 2018

LGTM!

Copy link
Contributor

@carlosms carlosms left a comment

Choose a reason for hiding this comment

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

I left a couple of comments but I think it's looking great as it is.

Makefile Outdated
@@ -68,6 +69,16 @@ build-path:

serve: | front-build back-start

compose-serve: | require-repos-folder front-dependencies build
REPOS_FOLDER=${REPOS_FOLDER} \
Copy link
Contributor

Choose a reason for hiding this comment

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

question: should this use the same GITBASEPG prefix as other env vars? It's not used by gitbase-playground itself, but it's used by the docker compose file we provide so... I'm not sure.


services:
playground:
image: gitbase-playground-from-sources
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: what about shorter gitbase-playground-dev?

@dpordomingo
Copy link
Contributor Author

@carlosms many thanks for your suggestions; they were addressed by d2e80c7 and 00eab08
Let me know if it looks good to you and I'll rebase and squash commits, and then proceed with the merge :D

@carlosms
Copy link
Contributor

👏 go ahead!

dpordomingo and others added 5 commits May 21, 2018 11:32
Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>
Signed-off-by: David Pordomingo <david.pordomingo.f@gmail.com>
Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>
Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>
Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>
Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>
@dpordomingo dpordomingo merged commit 2bb41f9 into src-d:master May 21, 2018
@dpordomingo dpordomingo deleted the docker-compose branch May 21, 2018 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants