X Tutup
The Wayback Machine - https://web.archive.org/web/20201015143022/https://github.com/certbot/certbot/pull/5435
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

Cleanup dockerfile-dev #5435

Merged
merged 3 commits into from Feb 16, 2018
Merged

Cleanup dockerfile-dev #5435

merged 3 commits into from Feb 16, 2018

Conversation

@bmw
Copy link
Member

@bmw bmw commented Jan 16, 2018

I'm using this currently and hit a number of snags. Changes are:

  • Upgrade to Xenial which has Python libraries with real SSL support.
  • Stop storing default Certbot directories as volumes as this is rarely (if ever) necessary in development.
  • Expose port 80 as well for testing internal web servers.
  • Install apache and nginx (this causes Augeas to be installed and makes Apache tests work).
  • Copy all files from the repo so tools continue to work as they begin to rely on more files.
  • Use tools/venv.sh which pins all dependency versions for increased stability and installs all plugins.
bmw added 2 commits Jan 16, 2018
@bmw bmw added the area: testing label Jan 16, 2018
@bmw bmw requested a review from joohoi Feb 12, 2018
Copy link
Member

@joohoi joohoi left a comment

LGTM in general. Approved the changes, and if the python3-dev is actually required, the PR is good to get merged.

apt-get install python3-dev git -y && \
COPY . .
RUN apt-get update && \
apt-get install apache2 git nginx-light python3-dev -y && \

This comment has been minimized.

@joohoi

joohoi Feb 16, 2018
Member

Is python3-dev needed here as we're effectively installing python2 venv?

This comment has been minimized.

@bmw

bmw Feb 16, 2018
Author Member

You're right. Nice catch. I thought we needed it to run Python 3 tests in tox, but cryptography's precompiled wheel saves us here. I've removed the package.

@joohoi
joohoi approved these changes Feb 16, 2018
Copy link
Member

@joohoi joohoi left a comment

LGTM!

@bmw bmw merged commit adec7a8 into master Feb 16, 2018
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.1%) to 98.671%
Details
@bmw bmw deleted the improve-dockerfile-dev branch Feb 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.
X Tutup