Skip to content
This repository has been archived by the owner on Jul 30, 2019. It is now read-only.

DO NOT MERGE: refresh Dockerfile a little #26

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rjw57
Copy link
Member

@rjw57 rjw57 commented Jul 6, 2018

DO NOT MERGE: This PR is for discussion.

As noted in #25, our production Dockerfile is not as flexible as it could be. Enable configuration or OAUTH2_... settings via environment variables and add our logging configuration.

Update the Dockerfile to be based on our common images.

I've tagged this as DO NOT MERGE because we do not currently have a test infrastructure for lookupproxy so I'm loathe to merge such a fundamental modification to the Dockerfile without some test deployment to check first.

As noted in '25, our production Dockerfile is not as flexible as it
could be. Enable configuration or OAUTH2_... settings via environment
variables and add our logging configuration.

Update the Dockerfile to be based on our common images.
@rjw57 rjw57 requested a review from a team July 6, 2018 14:33
@codecov-io
Copy link

Codecov Report

Merging #26 into master will decrease coverage by 0.87%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #26      +/-   ##
==========================================
- Coverage   94.95%   94.07%   -0.88%     
==========================================
  Files          25       25              
  Lines         753      760       +7     
==========================================
  Hits          715      715              
- Misses         38       45       +7
Impacted Files Coverage Δ
lookupproxy/settings/docker.py 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 458c0c5...42a7321. Read the comment docs.

@abrahammartin
Copy link
Member

What is the purpose of the changes?

@rjw57
Copy link
Member Author

rjw57 commented Jul 12, 2018

Primarily to allow us to use the production Docker image in our various webapps during development. This is the next biggest time cost for running CI jobs. That and we should start to make use of our shared images. This PR is sort of a "holding area" for the code change but, as outlined in the comment, I'm worried about merging before we have a test lookupproxy deployment.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants