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

Add python and pip to Dockerfile to support running notebooks on deepnote.com #15586

Merged
merged 1 commit into from
Aug 10, 2021

Conversation

RussTedrake
Copy link
Contributor

@RussTedrake RussTedrake commented Aug 9, 2021

The current drake docker images will not run on deepnote. Adding these lines fulfills the requirements, as specified here:
https://docs.deepnote.com/environment/custom-environments#custom-image-requirements , and seem a small price to pay for the increased capability.

To reproduce, I have uploaded the revised images to DockerHub as robotlocomotion/drake:pr15586-bionic and robotlocomotion/drake:pr15586-focal. Visit https://deepnote.com/project/Drake-PR-15586-mPXpOqUZTqe8JBtfj5z4xQ/%2Fnotebook.ipynb . You will need to create an account to proceed, I'm afraid. Then

  • On the left menu, click on the Environment toolbar, and then look at the "Environment" dropdown.
  • I've connected 4 docker images: robotlocomotion/drake:bionic and robotlocomotion/drake:focal and the two revisions.
  • Confirm that selecting the two existing images will spend some time "Loading image..." but then will fail with

We failed to run the image you specified. Your image doesn’t exist, or we don’t have permissions to access it, or it doesn't fulfill the requirements to run.

  • Confirm that selecting the revised images result in the images loading successfully.
  • For each of the revised images, run the first cell of the notebook, and see that pydrake loads successfully and prints the expected file path.

This change is Reviewable

Copy link
Contributor Author

@RussTedrake RussTedrake left a comment

Choose a reason for hiding this comment

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

+@BetsyMcPhail for feature review, please
+@EricCousineau-TRI for platform review, please

Reviewable status: LGTM missing from assignees BetsyMcPhail,EricCousineau-TRI(platform) (waiting on @BetsyMcPhail and @EricCousineau-TRI)

@jwnimmer-tri
Copy link
Collaborator

Instead of changing which python the Ubuntu system management daemons / cron jobs are going to use, we could also change only the user's path?

ln -s /usr/bin/python3 /opt/drake/bin/python
ln -s /usr/bin/pip3 /opt/drake/bin/pip

I wonder if that's sufficient for deepnote?

@jwnimmer-tri
Copy link
Collaborator

Alternatively, if supporting only 20.04 only is okay (why would deepnote need 18.04 in the first place?), then I think just sudo apt-get install python3-pip python-is-python3 would get you what you want in /usr/bin directly. Doing that (instead of alternatives games) on 20.04 seems best in any case, even if 18.04 still needs symlink tricks.

@RussTedrake
Copy link
Contributor Author

I'm ok with the 20.04 only solution. I'll need to update my class repos to 20.04, but I've been meaning to do it anyway.

…n deepnote.com

The current images will not run on deepnote.  Adding these lines fulfills the requirements, as specified here:
https://docs.deepnote.com/environment/custom-environments#custom-image-requirements

To reproduce, I have uploaded the revised image to DockerHub as robotlocomotion/drake:pr15586-focal.  Visit https://deepnote.com/project/Drake-PR-15586-mPXpOqUZTqe8JBtfj5z4xQ/%2Fnotebook.ipynb .  You will need to create an account to proceed, I'm afraid.  Then
- On the left menu, click on the Environment toolbar, and then look at the "Environment" dropdown.
- I've connected 3 docker images: robotlocomotion/drake:bionic and robotlocomotion/drake:focal and robotlocomotion/drake:pr15586-focal
- Confirm that selecting the two existing images will spend some time "Loading image..." but then will fail with

`We failed to run the image you specified. Your image doesn’t exist, or we don’t have permissions to access it, or it doesn't fulfill the requirements to run.`

- Confirm that selecting the revised image will load successfully.
- For the revised image, run the first cell of the notebook, and see that pydrake loads successfully and prints the expected file path.
Copy link
Contributor Author

@RussTedrake RussTedrake left a comment

Choose a reason for hiding this comment

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

I've made the updates (including to the commit message) and pushed the new images for testing. @jwnimmer-tri -- do you want to serve as platform, since you've already thought it through? @BetsyMcPhail -- can I get your feature review, please?

For completeness, these

ln -s /usr/bin/python3 /opt/drake/bin/python
ln -s /usr/bin/pip3 /opt/drake/bin/pip

were also sufficient for deepnote, but I've gone with the python-is-python3 spelling as requested.

Reviewable status: LGTM missing from assignees EricCousineau-TRI(platform),BetsyMcPhail (waiting on @BetsyMcPhail and @EricCousineau-TRI)

Copy link
Collaborator

@jwnimmer-tri jwnimmer-tri left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: LGTM missing from assignee BetsyMcPhail (waiting on @BetsyMcPhail)

Copy link
Contributor Author

@RussTedrake RussTedrake left a comment

Choose a reason for hiding this comment

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

-@EricCousineau-TRI for platform
+@jwnimmer-tri for platform, instead.

Reviewable status: LGTM missing from assignee BetsyMcPhail (waiting on @BetsyMcPhail)

@jwnimmer-tri
Copy link
Collaborator

@RussTedrake If you're happy to just have code review (i.e., I didn't do any manual testing of deepnote as part of my review), then I'd be okay with this PR as single-reviewer. I don't think adding two core ubuntu packages to the image is of any great risk of runtime errors nor developer confusion.

@RussTedrake
Copy link
Contributor Author

It certainly seems sufficient to me. I think @EricCousineau-TRI might have been taking it for a spin just now. If I don't hear more by this evening, then I'll push it through.

Copy link
Contributor

@EricCousineau-TRI EricCousineau-TRI left a comment

Choose a reason for hiding this comment

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

Tried to access w/ OAuth via GitHub and Google, but still couldn't access it.
That being said, I think you can merge w/o my review on the notebook / DeepNote workflow for now!

The simplified Docker changes also look great!
-@BetsyMcPhail +@EricCousineau-TRI :lgtm:

Reviewed 1 of 2 files at r2, all commit messages.
Reviewable status: :shipit: complete! all discussions resolved, LGTM from assignees jwnimmer-tri(platform),EricCousineau-TRI(platform) (waiting on @RussTedrake)

@jwnimmer-tri jwnimmer-tri merged commit 427778a into RobotLocomotion:master Aug 10, 2021
@RussTedrake RussTedrake deleted the deepnote branch October 11, 2021 10:43
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.

4 participants