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

Explicitly install redis-server when building test container #410

Merged
merged 2 commits into from
Feb 13, 2018

Conversation

Corey-Zumar
Copy link
Contributor

No description provided.

@@ -10,6 +10,9 @@ RUN DEBIAN_FRONTEND=noninteractive apt-get update && apt-get install --fix-missi
&& apt-get update \
&& apt-get install -y docker-ce

# install redis-server
RUN apt-get install -y redis-server
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of making a new layer why don't you just add it to the list of deps installed in lines 5-7

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was confused by the install docker header, but it appears that we're doing more than installing docker in that section. Updated the comment and moved the installation into that layer.

Copy link
Contributor

@dcrankshaw dcrankshaw left a comment

Choose a reason for hiding this comment

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

lgtm

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/995/
Test PASSed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/996/
Test PASSed.

@Corey-Zumar Corey-Zumar merged commit dd12718 into ucbrise:develop Feb 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants