-
Notifications
You must be signed in to change notification settings - Fork 5k
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
descriptive log for port unavailable and port-retries=0 #5136
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change looks good - thank you.
While we're on this topic and this area of the code, I think a similar change should be made to the info message within the for loop. That is, don't issue the "trying another port" clause when retries are not desired.
Also please edit your initial comment above to include |
Edited the comment, thank you. Is removing the |
I'm only suggesting the clause's removal when |
@kevin-bates - I have added the changes you suggested, in the PR. I made a new commit instead of rebasing, in order to highlight my new changes. I will rebase the commits if needed after you approve it. Thank you. |
71bc7b4
to
5996cdf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - thank you for you first contribution! 🎉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
when
--port-retries=0
and the launch fails on the port, the error logERROR: the notebook server could not be started because no available port could be found.
is not very appropriate. Fixes: #1914