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

Exposes CPU limit #579

Merged
merged 2 commits into from Feb 19, 2019

Conversation

@GladysNalvarte
Copy link
Contributor

GladysNalvarte commented Feb 15, 2019

Includes extra kwargs to limit CPU quota when building and running a docker image.

Closes #459

@betatim

This comment has been minimized.

Copy link
Member

betatim commented Feb 18, 2019

Nice work! I left a small comment on making the new argument a named parameter.

There are a few tests that fail that need fixing and then this can be merged I think. For test_cache_from_base we need to adjust the test but for others like test_connect_url I don't think we should edit the test and instead change the code of the repo2docker library itself to make the test work.

@@ -672,7 +671,7 @@ def build(self):
self.log.info(l['error'], extra=dict(phase='failure'))
raise docker.errors.BuildError(l['error'], build_log='')
elif 'status' in l:
self.log.info('Fetching base image...\r',
self.log.info('Fetching base image...\r',

This comment has been minimized.

Copy link
@betatim

betatim Feb 19, 2019

Member

Nice catch!

@@ -9,6 +9,8 @@
import sys
import xml.etree.ElementTree as ET

from traitlets import Dict

This comment has been minimized.

Copy link
@betatim

betatim Feb 19, 2019

Member

Do we need this import in this file?

@betatim betatim merged commit 9c25112 into jupyter:master Feb 19, 2019
3 checks passed
3 checks passed
ci/circleci: build_docs Your tests passed on CircleCI!
Details
ci/dockercloud Your tests passed in Docker Cloud
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@betatim

This comment has been minimized.

Copy link
Member

betatim commented Feb 19, 2019

Nice work! Merged this now, we can make a new PR to clean up the unused commit (if it is indeed unused).

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.