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

Improve jupyter launching (Jupyter Lab + enhanced alternate PORT) #438

Merged
merged 6 commits into from
Aug 3, 2021

Conversation

y2kbugger
Copy link
Contributor

What is the purpose of this PR?

  • Launch Jupyter as Jupyter Lab instead of Jupyter Notebook
  • Enhance start_docker.sh to pass the first available port to jupyter so that "remapped port" warning is no longer required.
  • Enhance available port detection to be more reliable
  • Add execute to start_docker.sh to allow running directly, updated readme to match.

How did you implement your changes

I've touched start_docker.sh and Dockerfile.
For details, see commit history as I have broken up commits to tell story clearly.

Remaining issues

None

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@y2kbugger
Copy link
Contributor Author

@ngreenwald Figured we could merge this into master prior to finalizing the metacluster GUI.

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.

Overall looks good! Is there a code cleanliness reason that having the script be executable is preferable to running bash scriptname? Or just a personal preference?

@y2kbugger
Copy link
Contributor Author

Overall looks good! Is there a code cleanliness reason that having the script be executable is preferable to running bash scriptname? Or just a personal preference?

I'd say it follows "least astonishment" since it allows both bash ./script.sh and ./script.shand different people might be used to one or another.
Marking as executable also made sense since the script already included a the "hash-bang" (#!), which does nothing if not ran directly.

start_docker.sh

#!/bin/bash
...

@y2kbugger
Copy link
Contributor Author

I did just push a small change, to use the slightly more portable #!/usr/bin/env bash ipo #!/bin/bash

https://stackoverflow.com/questions/10376206/what-is-the-preferred-bash-shebang

@ngreenwald ngreenwald self-requested a review August 3, 2021 04:25
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.

Got it, that all makes sense.

@ngreenwald ngreenwald merged commit 9ef485e into master Aug 3, 2021
@ngreenwald ngreenwald deleted the improve_jupyter_launch branch August 3, 2021 04:27
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.

2 participants