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

[MRG] Add caching of already built repositories #511

Merged
merged 1 commit into from Dec 22, 2018

Conversation

@betatim
Copy link
Member

betatim commented Dec 17, 2018

This PR is a revival of #461

@betatim betatim changed the title Add caching of already built repositories [WIP] Add caching of already built repositories Dec 17, 2018
@betatim betatim force-pushed the betatim:cached-builds-take2 branch from 41d61ae to e34d780 Dec 17, 2018
@betatim betatim changed the title [WIP] Add caching of already built repositories [MRG] Add caching of already built repositories Dec 17, 2018
@betatim betatim requested a review from yuvipanda Dec 17, 2018
@betatim

This comment has been minimized.

Copy link
Member Author

betatim commented Dec 17, 2018

Should we try again with this @yuvipanda? :)

@betatim betatim force-pushed the betatim:cached-builds-take2 branch from a471a5c to 9bc0905 Dec 17, 2018
return True
return False

def _build_image(self, checkout_path):

This comment has been minimized.

Copy link
@yuvipanda

yuvipanda Dec 17, 2018

Collaborator

I think this can be folded into the build function

This comment has been minimized.

Copy link
@betatim

betatim Dec 17, 2018

Author Member

I liked the separation because build() does a bunch of things before&after building, and _build_image() just builds the image. :-/ It is an attempt to make the functions a bit shorter. Happy to move it.

This comment has been minimized.

Copy link
@betatim

betatim Dec 19, 2018

Author Member

bump

This comment has been minimized.

Copy link
@betatim

betatim Dec 21, 2018

Author Member

Private conversation with @yuvipanda just now: we put everything back in one method and he is happy with that.

Maybe look at splitting at a later point.

Copy link
Collaborator

yuvipanda left a comment

Minor comment. I like the mocking!

@betatim betatim force-pushed the betatim:cached-builds-take2 branch 4 times, most recently from 031ce51 to 35fc757 Dec 19, 2018
Add tests for image caching

Adjust tests and main app for cached builds

Remove obsolete command-line handling

Remove print statement from test

Fix subdirectory handling

Put back exception instead of sys.exit()
@betatim betatim force-pushed the betatim:cached-builds-take2 branch from 35fc757 to e7018d7 Dec 21, 2018
@yuvipanda yuvipanda merged commit 18e9d2f into jupyter:master Dec 22, 2018
5 checks passed
5 checks passed
ci/circleci: build_docs Your tests passed on CircleCI!
Details
ci/dockercloud Your tests passed in Docker Cloud
Details
codecov/patch 100% of diff hit (target 20%)
Details
codecov/project 85.42% (+0.36%) compared to 8d26174
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@betatim betatim deleted the betatim:cached-builds-take2 branch Dec 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.