-
Notifications
You must be signed in to change notification settings - Fork 3k
Use tty for running docker commands by default #2260
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
Conversation
I first made our tests work fine without tty (and fixed a bug in our start script) and unified tests, but now I want to have tty by default:
|
no_warnings=False, | ||
user="1010", | ||
command=["id"], | ||
timeout=5, no_warnings=False, user="1010", command=["id"], tty=False |
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 is the only test which fails with tty enabled (I tested base-notebook locally).
It also fails when running manually:
docker run -it --user 1010 quay.io/jupyter/base-notebook:latest id
If I remove -t
, then it gives expected output.
The source of /usr/local/bin/before-notebook.d/10activate-conda-env.sh unfortunately doesn't work
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.
Got it, I removed the script in the image, and then ran it's content manually (eval "$(conda shell.bash hook)"
) and in the end it gives:
Would you like conda to send this report to the core maintainers? [y/N]:
So in a non-interactive mode it just finishes, but in interactive it fails after timeout.
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.
And it fails after 40 seconds which is more than our timeout, that's why test reports as failed
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.
Setting report_errors in config won't work here, because in this test the /home/jovyan
is not accessible.
So the best we can do is to put a link why we need to disable tty there, at least now I know the reason.
Describe your changes
Issue ticket if applicable
Checklist (especially for first-time contributors)