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

Notebook update fix and external drive compatibility #478

Merged
merged 8 commits into from
Oct 25, 2021

Conversation

ackagel
Copy link
Contributor

@ackagel ackagel commented Oct 19, 2021

Purpose

Fixes issue #477 and adds configurable external drive compatibility via the --external '/path/to/drive' or -e '/path/to/drive' argument flags.

Implementation

Moves some logic from update_notebooks.sh into start_docker.sh so that update_notebooks.sh is called conditionally. As for external drive compatibility, that just needs to be added to docker's filepaths in the preferences menu.

Remaining issues

The --external tag takes a path as a second argument in order to be compatible with multiple OS's. On macOS, this would always be /Volumes, while on Windows, it could be G:\\, F:\\, etc. I could see this being kind of annoying for macOS users, so a separate --mount_volumes | -m option could be added.

Copy link
Member

@ngreenwald ngreenwald 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. Did you test that update now works as intended, and that the external path works? What happens if the external path that is supplied has an error in it, or hasn't been added to docker preferences?

Copy link
Contributor

@alex-l-kong alex-l-kong left a comment

Choose a reason for hiding this comment

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

Not much else to add on top of Noah's comments.

@ackagel
Copy link
Contributor Author

ackagel commented Oct 21, 2021

@ngreenwald update works as intended, and the external path works great for macOS. As for Windows, it's actually easier with the WSL2 backend, in that you don't need to share specific files in the Preferences menu. Updated the readme with that info.

Here's the consequence of a bad path on macOS:

Screen Shot 2021-10-21 at 11 09 18 AM

@ngreenwald ngreenwald merged commit 5b248e5 into master Oct 25, 2021
@ngreenwald ngreenwald deleted the start-docker-upgrade branch October 25, 2021 22:05
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.

3 participants