Skip to content

Conversation

@robnagler
Copy link
Contributor

Specifically, to support overriding terminado's shell_command with something that allows flags. We are using JupyterHub, and it's important for our users to get a clean shell (bash --login).

"Jupyter notebook uses.")

terminado_settings = Dict(config=True,
help='Supply overrides for terminado. Currently only supports "shell_command".')
Copy link
Member

Choose a reason for hiding this comment

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

Are there other things you envisage putting in here? Would it be simpler to have terminals_shell_command as a configurable thing by itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't want to add anything I didn't need right now. I could see someone wanting to add environment variables or , for instance. It also seems to me that it is also consistent with tornado_settings.

Copy link
Member

Choose a reason for hiding this comment

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

It's a reasonable enough approach. I think tornado_settings is a bit of a special case because there are so many things that might go in there, but I'd be happy either way. Let's give others a chance to look at this.

@minrk
Copy link
Member

minrk commented May 18, 2016

Since you made this PR from your master branch, it looks like it's pulling in work from other PRs. Do you want to get this back to just the proposed change?

@robnagler
Copy link
Contributor Author

I'll resubmit. Not sure how to remove commits from an existing PR.

@robnagler robnagler closed this May 18, 2016
@minrk minrk added this to the no action milestone May 20, 2016
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants