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

Move docker dep commands to earlier in the build #209

Merged

Conversation

dekimsey
Copy link
Contributor

@dekimsey dekimsey commented Jul 11, 2019

Description

This will let Docker cache the results of the vendor dependencies.
Making re-builds during testing faster.

I also changed the logic to just rm -f the .env which silences a spurious error message I was encountering.

Step 9/15 : RUN ./configure && make build && touch jwt_signing_key.pem
 ---> Running in c77e9d8bca8d
rm: cannot remove '.env': No such file or directory
Checking for make... found
Checking for awk... found
Checking for go... found

Motivation and Context

To test my changes, I'm just running docker build . repeatedly. I found the dep ensure command took a long time to do it's work.

How Has This Been Tested?

It builds without triggering re-downloading of vendor artifacts.

Checklist:

  • My change requires a change to the documentation or CHANGELOG.
  • I have updated the documentation/CHANGELOG accordingly.
  • I have created a feature (non-master) branch for my PR.

@dekimsey dekimsey requested a review from a team July 11, 2019 22:22
Copy link
Member

@JoelSpeed JoelSpeed left a comment

Choose a reason for hiding this comment

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

Looks good!

Could you please answer my question below and add an update to the changelog please

configure Show resolved Hide resolved
@dekimsey dekimsey mentioned this pull request Jul 15, 2019
3 tasks
@dekimsey dekimsey force-pushed the improve-docker-rebuild-caching branch from ec71678 to c82dbd1 Compare July 15, 2019 14:56
JoelSpeed
JoelSpeed previously approved these changes Jul 15, 2019
Copy link
Member

@JoelSpeed JoelSpeed left a comment

Choose a reason for hiding this comment

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

I'm happy with this, thanks for the PR

configure Show resolved Hide resolved
This will let Docker cache the results of the vendor dependencies.
Making re-builds during testing faster.

Also clean-up spurious test & rm in ./configure
@dekimsey dekimsey force-pushed the improve-docker-rebuild-caching branch from c82dbd1 to 816c2a6 Compare July 15, 2019 15:00
@dekimsey
Copy link
Contributor Author

I squashed all the changes. Feel free to merge when ready.

@JoelSpeed JoelSpeed merged commit e952ab4 into oauth2-proxy:master Jul 15, 2019
Jing-ze pushed a commit to Jing-ze/oauth2-proxy that referenced this pull request Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants