-
Notifications
You must be signed in to change notification settings - Fork 37
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
Gh-334: Smoother gaffer gremlin deployment #337
Gh-334: Smoother gaffer gremlin deployment #337
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency I don't think we should be adding a new Dockerfile with a different approach to building compared to what has been done before, especially as the existing Dockerfile can simply be reused.
I would suggest keeping the existing structure and not adding the new gremlin-gaffer-demo
directory, but instead create a directory under gaffer-gremlin
for the demo. These can then use docker-compose for build/deployment, as done with all other images.
I'd suggest this layout
gaffer-gremlin
├── demo
│ ├── docker-compose.yaml to run the demo
│ └── other files for demo use (inc README)
├── proxy
│ ├── docker-compose.yaml to run the proxy
│ └── other files for proxy use (inc README)
├── Dockerfile
└── High level README.md
Or the proxy config could sit in the root directory for simplicity as this would be the more common usage.
Missed this comment before committing but changes implemented pretty much are along these lines, moving into an |
I agree with tb here that we shouldn't be hardcoding URLs. If we change anything in the future we would have to change all the files which isn't best practice. Maven should do the heavy lifting. After all it is a dependency manager for a reason. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the build failure is unrelated to this PR. But it might be best if it is fixed before this is merged.
Adds a new
gaffer-gremlin
container image that comes pre-configured to run a gremlin server with the gafferpop library using a proxy store to point at an existing Gaffer instance. Now the gaffer docs have been updated too there is a guide for users on configuring this container.Also integrated the image build into the CI and tidied a few bits up.
Related issue