-
-
Notifications
You must be signed in to change notification settings - Fork 2k
Fix dokku ps <app> over ssh #1419
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 don't understand the help dialog change. Can you explain that a bit more? |
|
Happy to. If you read it literally, just like a brand new user may, it asks you (in quotes) to type 'dokku help'. Here's a sample interaction before PR: foo
! `foo` is not a dokku command.
! See `dokku help` for a list of available commands.
dokku help
! `dokku help` is not a dokku command.
! See `dokku help` for a list of available commands.😦 foo
! `foo` is not a dokku command.
! See `help` for a list of available commands.
help
Usage: dokku [--quiet|--trace|--rm-container|--rm|--force] COMMAND <app> [command-specific-options]
Options:
apps:cre...😄 |
|
Wait, are you inside of |
|
Correct, aka |
|
I edited my example above to clarify. |
|
I think the expectation is that once you are in a dokku shell, you don't need to use the prefix anymore. Can you revert that change in your PR (and squash the commits)? You can open a second PR adding custom MOTD - telling people to not prefix dokku commands - for dokku shell, which will probably be much more useful than this change. There are other places where we call out the |
Resolved the TTY error when connecting over ssh. Updated ps to use has_tty function.
|
np, just pushed up the change. |
|
Wait, why did you get a tty error before? Note: I don't seem to have such an error on master. Can you paste the command that results in an error? |
|
Here you go: $ ssh dokku@dokku.me ps node-js-app
time="2015-08-26T21:58:25Z" level=fatal msg="cannot enable tty mode on non tty input" Do you have If so, you may not see this bug. Because that line is buried in the documentation, this brings up a new question: Should we just bubble up this documentation since this would be bug is probably lurking in other sub-commands in Dokku or should we fix the TTY bugs (since there is an easy function for it already) when we find them? |
|
Here is my |
|
Hmm somewhere that's being set. Try disabling it with -T, my patch allows you to execute without the pseudo-tty: $ ssh -T dokku@dokku.me ps node-js-app |
|
Maybe it's because I'm using |
|
Given the variance in how different folks might have configured their SSH clients, I like the idea of using |
|
@michaelshobbs I haven't read up about the flag just yet, but if you feel comfortable merging, ship it :) |
|
@michaelshobbs is correct. This patch removes the requirement. Yep, I tested dokku_client.sh and it does work with or without the patch. |
Fix dokku ps <app> over ssh
|
@u2mejc good work! Mind making a PR to add MOTD support to |
Corrects error dialog seen by new users attempting to use the shell's help:
dokku> dokku help
!
dokku helpis not a dokku command.! See
dokku helpfor a list of available commands.Resolves the TTY error when connecting over ssh. Updated ps to use has_tty function.