Skip to content

Commit

Permalink
Flush stdout after input prompt.
Browse files Browse the repository at this point in the history
Some terminal emulators (such as mintty) will not write output to the
terminal window until stdout is explicitly flushed. When running `aws
configure` a user would see nothing but a new line. After pressing
'enter' four times, they would then see the output flushed all in a
single line. This makes it very difficult to configure the command.

The solution is to call `flush` after every prompt. Since `raw_input`
does not have an option to do this, we have to prompt and flush
manually. Since we're already accessing stdout directly, it's easier to
write to it directly since print adds some formatting that we don't
want (namely, a newline at the end of the print).

Fixes aws#1925
  • Loading branch information
JordonPhillips committed May 2, 2016
1 parent fdb4c27 commit c528fc8
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 10 deletions.
5 changes: 5 additions & 0 deletions .changes/next-release/bugfix-Configure.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"type": "bugfix",
"description": "Fix issue causing prompts not to display on mintty. Fixes `#1925 <https://github.com/aws/aws-cli/issues/1925>`__",
"category": "Configure"
}
17 changes: 17 additions & 0 deletions awscli/compat.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,3 +105,20 @@ def compat_open(filename, mode='r', encoding=None):
if 'b' not in mode:
encoding = locale.getpreferredencoding()
return io.open(filename, mode, encoding=encoding)


def compat_input(prompt):
"""
Cygwin's pty's are based on pipes. Therefore, when it interacts with a Win32
program (such as Win32 python), what that program sees is a pipe instead of
a console. This is important because python buffers pipes, and so on a
pty-based terminal, text will not necessarily appear immediately. In most
cases, this isn't a big deal. But when we're doing an interactive prompt,
the result is that the prompts won't display until we fill the buffer. Since
raw_input does not flush the prompt, we need to manually write and flush it.
See https://github.com/mintty/mintty/issues/56 for more details.
"""
sys.stdout.write(prompt)
sys.stdout.flush()
return raw_input()
4 changes: 2 additions & 2 deletions awscli/customizations/configure/configure.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

from botocore.exceptions import ProfileNotFound

from awscli.compat import raw_input
from awscli.compat import compat_input
from awscli.customizations.commands import BasicCommand
from awscli.customizations.configure.addmodel import AddModelCommand
from awscli.customizations.configure.set import ConfigureSetCommand
Expand All @@ -39,7 +39,7 @@ class InteractivePrompter(object):
def get_value(self, current_value, config_name, prompt_text=''):
if config_name in ('aws_access_key_id', 'aws_secret_access_key'):
current_value = mask_value(current_value)
response = raw_input("%s [%s]: " % (prompt_text, current_value))
response = compat_input("%s [%s]: " % (prompt_text, current_value))
if not response:
# If the user hits enter, we return a value of None
# instead of an empty string. That way we can determine
Expand Down
41 changes: 33 additions & 8 deletions tests/unit/customizations/configure/test_configure.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

from awscli.customizations.configure import configure, ConfigValue, NOT_SET
from awscli.testutils import unittest
from awscli.compat import six

from . import FakeSession

Expand Down Expand Up @@ -132,12 +133,15 @@ def test_session_says_profile_does_not_exist(self):
class TestInteractivePrompter(unittest.TestCase):

def setUp(self):
self.patch = mock.patch(
'awscli.customizations.configure.configure.raw_input')
self.mock_raw_input = self.patch.start()
self.input_patch = mock.patch('awscli.compat.raw_input')
self.mock_raw_input = self.input_patch.start()
self.stdout = six.StringIO()
self.stdout_patch = mock.patch('sys.stdout', self.stdout)
self.stdout_patch.start()

def tearDown(self):
self.patch.stop()
self.input_patch.stop()
self.stdout_patch.stop()

def test_access_key_is_masked(self):
self.mock_raw_input.return_value = 'foo'
Expand All @@ -148,7 +152,7 @@ def test_access_key_is_masked(self):
# First we should return the value from raw_input.
self.assertEqual(response, 'foo')
# We should also not display the entire access key.
prompt_text = self.mock_raw_input.call_args[0][0]
prompt_text = self.stdout.getvalue()
self.assertNotIn('myaccesskey', prompt_text)
self.assertRegexpMatches(prompt_text, r'\[\*\*\*\*.*\]')

Expand All @@ -160,7 +164,7 @@ def test_access_key_not_masked_when_none(self):
prompt_text='Access key')
# First we should return the value from raw_input.
self.assertEqual(response, 'foo')
prompt_text = self.mock_raw_input.call_args[0][0]
prompt_text = self.stdout.getvalue()
self.assertIn('[None]', prompt_text)

def test_secret_key_is_masked(self):
Expand All @@ -170,7 +174,7 @@ def test_secret_key_is_masked(self):
config_name='aws_secret_access_key',
prompt_text='Secret Key')
# We should also not display the entire secret key.
prompt_text = self.mock_raw_input.call_args[0][0]
prompt_text = self.stdout.getvalue()
self.assertNotIn('mysupersecretkey', prompt_text)
self.assertRegexpMatches(prompt_text, r'\[\*\*\*\*.*\]')

Expand All @@ -180,7 +184,7 @@ def test_non_secret_keys_are_not_masked(self):
current_value='mycurrentvalue', config_name='not_a_secret_key',
prompt_text='Enter value')
# We should also not display the entire secret key.
prompt_text = self.mock_raw_input.call_args[0][0]
prompt_text = self.stdout.getvalue()
self.assertIn('mycurrentvalue', prompt_text)
self.assertRegexpMatches(prompt_text, r'\[mycurrentvalue\]')

Expand All @@ -196,6 +200,27 @@ def test_user_hits_enter_returns_none(self):
# was no input.
self.assertIsNone(response)

def test_compat_input_flushes_after_each_prompt(self):
# Clear out the default patch
self.stdout_patch.stop()

# Create a mock stdout to record flush calls and replace stdout_patch
self.stdout = mock.Mock()
self.stdout_patch = mock.patch('sys.stdout', self.stdout)
self.stdout_patch.start()

# Make sure flush called at least once
prompter = configure.InteractivePrompter()
prompter.get_value(current_value='foo', config_name='bar',
prompt_text='baz')
self.assertTrue(self.stdout.flush.called)

# Make sure flush is called after *every* prompt
self.stdout.reset_mock()
prompter.get_value(current_value='foo2', config_name='bar2',
prompt_text='baz2')
self.assertTrue(self.stdout.flush.called)


class TestConfigValueMasking(unittest.TestCase):

Expand Down

0 comments on commit c528fc8

Please sign in to comment.