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.

Sign up for GitHub

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

Jump to bottom

Centralized interactive input() function that checks if STDIN is a TTY #1856

Merged
merged 2 commits into from
Jan 25, 2017
Merged

Centralized interactive input() function that checks if STDIN is a TTY #1856

merged 2 commits into from
Jan 25, 2017

Conversation

derekbekoe
Copy link
Member

  • Promote _prompting to prompting
  • Use prompt() to check TTY before getting user input

Closes #1349

@tjprescott
Copy link
Member

Can we have a wrapper on getpass that confirms the typed password? I'd rather do that than roll my own in VM/VMSS create.

@derekbekoe
Copy link
Member Author

User should always type the password twice?

troydai
troydai approved these changes Jan 25, 2017
View reviewed changes
src/azure-cli-core/azure/cli/core/commands/__init__.py
@@ -19,7 +19,7 @@
import azure.cli.core.telemetry as telemetry
from azure.cli.core._util import CLIError
from azure.cli.core.application import APPLICATION
from azure.cli.core._prompting import prompt_y_n, NoTTYException
from azure.cli.core.prompting import prompt_y_n, NoTTYException
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you!

src/command_modules/azure-cli-configure/azure/cli/command_modules/configure/custom.py
@@ -6,15 +6,15 @@
from __future__ import print_function
import os
import sys
from six.moves import input, configparser #pylint: disable=redefined-builtin
from six.moves import configparser #pylint: disable=redefined-builtin
Copy link
Contributor

Choose a reason for hiding this comment

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

PEP8: two spaces between the '#' and the previous char, one space between the '#' and the following char.

@tjprescott
Copy link
Member

I would argue that yes, the user should always enter the password twice, but you could parameterize it with a default of False. The idea is that 1) the portal and like 99% of regular applications do this and 2) it would be very annoying to mistype your password and find you have no ability to access the VM you just waited to create because of a password typo. This at least ensures you got your password right or made the same mistake twice (in which case, no one can help you!)

tjprescott
tjprescott suggested changes Jan 25, 2017
View reviewed changes
Copy link
Member

@tjprescott tjprescott left a comment

Choose a reason for hiding this comment

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

Password prompt (even if opt-in) pleeease :)

@derekbekoe
Centralized interactive input() function that checks if STDIN is a TTY
bec96ea 
- Promote _prompting to prompting
- Use prompt() to check TTY before getting user input
+ fix pep8 spacing
@derekbekoe
Centralized password prompting with confirmation option
079f695
@derekbekoe
Copy link
Member Author

@tjprescott Added prompt_pass with confirmation option.

@tjprescott
Copy link
Member

Thanks @derekbekoe !

@derekbekoe derekbekoe merged commit 6224ec0 into Azure:master Jan 25, 2017
@derekbekoe derekbekoe deleted the central-prompt branch January 25, 2017 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tjprescott tjprescott tjprescott approved these changes

@troydai troydai troydai approved these changes

Assignees
No one assigned
Labels
cla-not-required
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@derekbekoe @tjprescott @troydai @azurecla @haroldrandom