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] Do not try to build the image with root as the primary user. #676

Merged
merged 1 commit into from May 8, 2019

Conversation

@Xarthisius
Copy link
Contributor

Xarthisius commented May 7, 2019

Fixes #267 and #395. Solution suggested in #267 (comment)

@Xarthisius Xarthisius force-pushed the Xarthisius:no_root branch from 5adee9f to 7242c9c May 7, 2019
@betatim
betatim approved these changes May 7, 2019
Copy link
Member

betatim left a comment

Thanks! Waiting for travis.

@Xarthisius

This comment has been minimized.

Copy link
Contributor Author

Xarthisius commented May 7, 2019

Did I draw a short straw and ubuntu removed artful within past few days/hours?

@betatim

This comment has been minimized.

Copy link
Member

betatim commented May 7, 2019

I think it is more likely this was a temporary connection thing as some of the tests passed that use the same docker file 🤞.

@betatim

This comment has been minimized.

Copy link
Member

betatim commented May 7, 2019

Though there is no good reason to use artful here, so we should switch them to bionic which we use for the rest of repo2docker.

@Xarthisius

This comment has been minimized.

Copy link
Contributor Author

Xarthisius commented May 7, 2019

I think it is more likely this was a temporary connection thing as some of the tests passed that use the same docker file .

I can reproduce that locally: docker run --rm -ti ubuntu:artful apt-get update -qqy. I think it's gone, gone. I will bump to bionic.

@betatim

This comment has been minimized.

Copy link
Member

betatim commented May 7, 2019

lol. It looks like the first test "passes" because we expect it to fail and it does, just not because of overallocation of RAM but because there is a problem with building the image. This means your suggestion of "artful support stopped" is probably right. I'll make a PR to switch the tests to bionic. I'll wait for you to do this.

@Xarthisius

This comment has been minimized.

Copy link
Contributor Author

Xarthisius commented May 7, 2019

lol. It looks like the first test "passes" because we expect it to fail and it does, just not because of overallocation of RAM but because there is a problem with building the image. This means your suggestion of "artful support stopped" is probably right. I'll make a PR to switch the tests to bionic.

OK, will wait for your PR then.

@betatim

This comment has been minimized.

Copy link
Member

betatim commented May 7, 2019

#677 for updating the base image.

Fixes #267 and #395
@Xarthisius Xarthisius force-pushed the Xarthisius:no_root branch from 7242c9c to 7d948bd May 8, 2019
@betatim

This comment has been minimized.

Copy link
Member

betatim commented May 8, 2019

#677 is merged. If we rebase this PR we should be good to go.

This PR is a great example of why it is so hard to estimate how long it will take to do something. "Change up a few lines to give a better error message, child's play!" ... except then we go down the rabbit hole of Ubuntu releases and how the Docker API works :)

@Xarthisius

This comment has been minimized.

Copy link
Contributor Author

Xarthisius commented May 8, 2019

#677 is merged. If we rebase this PR we should be good to go.

Already did that :)

@betatim

This comment has been minimized.

Copy link
Member

betatim commented May 8, 2019

Ah yes, I should make use of GitHub snitching on the git push -f crowd. It used to be that well kept secret between me and github when I did that ... and now they tell everyone! :)

@betatim betatim merged commit 4514ba3 into jupyter:master May 8, 2019
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 90.97% (+0.03%) compared to 2086b7a
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
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.