Repository: softlayer/softlayer-python

Issue:
[Bug] slcli config setup doesnt accept default value when one is already present
When you already have a `.softlayer` configured, and you `slcli config setup`, it loads previous values from `.softlayer` and shows them to you, but pressing enter at the prompt doesnt keep the previous value and move on to the next setting, it sits there and forces you to retype it and it shouldnt do that.

It also doesnt prompt for a timeout value, it should prompt and accept no value provided as meaning dont set and use default.

Using 4.0.0

Discussion:

Patch:
diff --git a/SoftLayer/CLI/config/setup.py b/SoftLayer/CLI/config/setup.py
index 30ba070a..f9b37d55 100644
--- a/SoftLayer/CLI/config/setup.py
+++ b/SoftLayer/CLI/config/setup.py
@@ -13,14 +13,12 @@
 import click
 
 
-def get_api_key(client, username, secret, endpoint_url=None):
+def get_api_key(client, username, secret):
     """Attempts API-Key and password auth to get an API key.
 
     This will also generate an API key if one doesn't exist
     """
 
-    client.endpoint_url = endpoint_url
-    client.auth = None
     # Try to use a client with username/api key
     if len(secret) == 64:
         try:
@@ -50,8 +48,8 @@ def cli(env):
 
     username, secret, endpoint_url, timeout = get_user_input(env)
 
-    api_key = get_api_key(env.client, username, secret,
-                          endpoint_url=endpoint_url)
+    env.client.transport.transport.endpoint_url = endpoint_url
+    api_key = get_api_key(env.client, username, secret)
 
     path = '~/.softlayer'
     if env.config_file:
@@ -79,6 +77,7 @@ def cli(env):
     parsed_config.set('softlayer', 'username', username)
     parsed_config.set('softlayer', 'api_key', api_key)
     parsed_config.set('softlayer', 'endpoint_url', endpoint_url)
+    parsed_config.set('softlayer', 'timeout', timeout)
 
     config_fd = os.fdopen(os.open(config_path,
                                   (os.O_WRONLY | os.O_CREAT | os.O_TRUNC),
@@ -96,47 +95,28 @@ def get_user_input(env):
     """Ask for username, secret (api_key or password) and endpoint_url."""
 
     defaults = config.get_settings_from_client(env.client)
-    timeout = defaults['timeout']
 
     # Ask for username
-    for _ in range(3):
-        username = (env.input('Username [%s]: ' % defaults['username']) or
-                    defaults['username'])
-        if username:
-            break
-    else:
-        raise exceptions.CLIAbort('Aborted after 3 attempts')
+    username = env.input('Username', default=defaults['username'])
 
     # Ask for 'secret' which can be api_key or their password
-    for _ in range(3):
-        secret = (env.getpass('API Key or Password [%s]: '
-                              % defaults['api_key']) or
-                  defaults['api_key'])
-        if secret:
-            break
-    else:
-        raise exceptions.CLIAbort('Aborted after 3 attempts')
+    secret = env.getpass('API Key or Password', default=defaults['api_key'])
 
     # Ask for which endpoint they want to use
-    for _ in range(3):
-        endpoint_type = env.input(
-            'Endpoint (public|private|custom): ')
-        endpoint_type = endpoint_type.lower()
-        if not endpoint_type:
-            endpoint_url = SoftLayer.API_PUBLIC_ENDPOINT
-            break
-        if endpoint_type == 'public':
-            endpoint_url = SoftLayer.API_PUBLIC_ENDPOINT
-            break
-        elif endpoint_type == 'private':
-            endpoint_url = SoftLayer.API_PRIVATE_ENDPOINT
-            break
-        elif endpoint_type == 'custom':
-            endpoint_url = env.input(
-                'Endpoint URL [%s]: ' % defaults['endpoint_url']
-            ) or defaults['endpoint_url']
-            break
-    else:
-        raise exceptions.CLIAbort('Aborted after 3 attempts')
+    endpoint_type = env.input(
+        'Endpoint (public|private|custom)', default='public')
+    endpoint_type = endpoint_type.lower()
+    if endpoint_type is None:
+        endpoint_url = SoftLayer.API_PUBLIC_ENDPOINT
+    if endpoint_type == 'public':
+        endpoint_url = SoftLayer.API_PUBLIC_ENDPOINT
+    elif endpoint_type == 'private':
+        endpoint_url = SoftLayer.API_PRIVATE_ENDPOINT
+    elif endpoint_type == 'custom':
+        endpoint_url = env.input('Endpoint URL',
+                                 default=defaults['endpoint_url'])
+
+    # Ask for timeout
+    timeout = env.input('Timeout', default=defaults['timeout'] or 0)
 
     return username, secret, endpoint_url, timeout
diff --git a/SoftLayer/CLI/core.py b/SoftLayer/CLI/core.py
index d2278394..a6839522 100644
--- a/SoftLayer/CLI/core.py
+++ b/SoftLayer/CLI/core.py
@@ -138,17 +138,19 @@ def cli(ctx,
     if env.client is None:
         # Environment can be passed in explicitly. This is used for testing
         if fixtures:
-            transport = SoftLayer.FixtureTransport()
+            client = SoftLayer.BaseClient(
+                transport=SoftLayer.FixtureTransport(),
+                auth=None,
+            )
         else:
             # Create SL Client
-            transport = SoftLayer.XmlRpcTransport()
-
-        wrapped_transport = SoftLayer.TimingTransport(transport)
-        env.client = SoftLayer.create_client_from_env(
-            proxy=proxy,
-            config_file=config,
-            transport=wrapped_transport,
-        )
+            client = SoftLayer.create_client_from_env(
+                proxy=proxy,
+                config_file=config,
+            )
+
+        client.transport = SoftLayer.TimingTransport(client.transport)
+        env.client = client
 
 
 @cli.resultcallback()
diff --git a/SoftLayer/CLI/environment.py b/SoftLayer/CLI/environment.py
index b67b8c24..f5800590 100644
--- a/SoftLayer/CLI/environment.py
+++ b/SoftLayer/CLI/environment.py
@@ -44,13 +44,13 @@ def fmt(self, output):
         """Format output based on current the environment format."""
         return formatting.format_output(output, fmt=self.format)
 
-    def input(self, prompt):
+    def input(self, prompt, default=None):
         """Provide a command prompt."""
-        return click.prompt(prompt)
+        return click.prompt(prompt, default=default)
 
-    def getpass(self, prompt):
+    def getpass(self, prompt, default=None):
         """Provide a password prompt."""
-        return click.prompt(prompt, hide_input=True)
+        return click.prompt(prompt, hide_input=True, default=default)
 
     # Command loading methods
     def list_commands(self, *path):
diff --git a/SoftLayer/CLI/formatting.py b/SoftLayer/CLI/formatting.py
index 1f7e495a..f1554af0 100644
--- a/SoftLayer/CLI/formatting.py
+++ b/SoftLayer/CLI/formatting.py
@@ -160,27 +160,6 @@ def transaction_status(transaction):
         transaction['transactionStatus'].get('friendlyName'))
 
 
-def valid_response(prompt, *valid):
-    """Prompt user for input.
-
-    Will display a prompt for a command-line user. If the input is in the
-    valid given valid list then it will return True. Otherwise, it will
-    return False. If no input is received from the user, None is returned
-    instead.
-
-    :param string prompt: string prompt to give to the user
-    :param string \\*valid: valid responses
-    """
-    ans = click.prompt(prompt).lower()
-
-    if ans in valid:
-        return True
-    elif ans == '':
-        return None
-
-    return False
-
-
 def confirm(prompt_str, default=False):
     """Show a confirmation prompt to a command-line user.
 
@@ -188,16 +167,17 @@ def confirm(prompt_str, default=False):
     :param bool default: Default value to True or False
     """
     if default:
+        default_str = 'y'
         prompt = '%s [Y/n]' % prompt_str
     else:
+        default_str = 'n'
         prompt = '%s [y/N]' % prompt_str
 
-    response = valid_response(prompt, 'y', 'yes', 'yeah', 'yup', 'yolo')
-
-    if response is None:
-        return default
+    ans = click.prompt(prompt, default=default_str, show_default=False)
+    if ans.lower() in ('y', 'yes', 'yeah', 'yup', 'yolo'):
+        return True
 
-    return response
+    return False
 
 
 def no_going_back(confirmation):
@@ -209,10 +189,14 @@ def no_going_back(confirmation):
     if not confirmation:
         confirmation = 'yes'
 
-    return valid_response(
-        'This action cannot be undone! '
-        'Type "%s" or press Enter to abort' % confirmation,
-        str(confirmation))
+    prompt = ('This action cannot be undone! Type "%s" or press Enter '
+              'to abort' % confirmation)
+
+    ans = click.confirm(prompt, default='', show_default=False).lower()
+    if ans == str(confirmation):
+        return True
+
+    return False
 
 
 class SequentialOutput(list):
diff --git a/SoftLayer/config.py b/SoftLayer/config.py
index 8c679e6c..120cadab 100644
--- a/SoftLayer/config.py
+++ b/SoftLayer/config.py
@@ -16,9 +16,12 @@ def get_client_settings_args(**kwargs):
 
         :param \\*\\*kwargs: Arguments that are passed into the client instance
     """
+    timeout = kwargs.get('timeout')
+    if timeout is not None:
+        timeout = float(timeout)
     return {
         'endpoint_url': kwargs.get('endpoint_url'),
-        'timeout': float(kwargs.get('timeout') or 0),
+        'timeout': timeout,
         'proxy': kwargs.get('proxy'),
         'username': kwargs.get('username'),
         'api_key': kwargs.get('api_key'),


Test patch:
diff --git a/SoftLayer/tests/CLI/environment_tests.py b/SoftLayer/tests/CLI/environment_tests.py
index e1ebf0e2..b3826df8 100644
--- a/SoftLayer/tests/CLI/environment_tests.py
+++ b/SoftLayer/tests/CLI/environment_tests.py
@@ -43,13 +43,13 @@ def test_get_command(self):
     @mock.patch('click.prompt')
     def test_input(self, prompt_mock):
         r = self.env.input('input')
-        prompt_mock.assert_called_with('input')
+        prompt_mock.assert_called_with('input', default=None)
         self.assertEqual(prompt_mock(), r)
 
     @mock.patch('click.prompt')
     def test_getpass(self, prompt_mock):
         r = self.env.getpass('input')
-        prompt_mock.assert_called_with('input', hide_input=True)
+        prompt_mock.assert_called_with('input', default=None, hide_input=True)
         self.assertEqual(prompt_mock(), r)
 
     def test_resolve_alias(self):
diff --git a/SoftLayer/tests/CLI/helper_tests.py b/SoftLayer/tests/CLI/helper_tests.py
index 80ae0bd6..d75e8e2b 100644
--- a/SoftLayer/tests/CLI/helper_tests.py
+++ b/SoftLayer/tests/CLI/helper_tests.py
@@ -41,50 +41,21 @@ def test_fail(self):
 
 class PromptTests(testing.TestCase):
 
-    @mock.patch('click.prompt')
-    def test_invalid_response(self, prompt_mock):
-        prompt_mock.return_value = 'y'
-        result = formatting.valid_response('test', 'n')
-        prompt_mock.assert_called_with('test')
-        self.assertFalse(result)
-
-        prompt_mock.return_value = 'wakakwakwaka'
-        result = formatting.valid_response('test', 'n')
-        prompt_mock.assert_called_with('test')
-        self.assertFalse(result)
-
-        prompt_mock.return_value = ''
-        result = formatting.valid_response('test', 'n')
-        prompt_mock.assert_called_with('test')
-        self.assertEqual(result, None)
-
-    @mock.patch('click.prompt')
-    def test_valid_response(self, prompt_mock):
-        prompt_mock.return_value = 'n'
-        result = formatting.valid_response('test', 'n')
-        prompt_mock.assert_called_with('test')
-        self.assertTrue(result)
-
-        prompt_mock.return_value = 'N'
-        result = formatting.valid_response('test', 'n')
-        prompt_mock.assert_called_with('test')
-        self.assertTrue(result)
-
-    @mock.patch('click.prompt')
-    def test_do_or_die(self, prompt_mock):
+    @mock.patch('click.confirm')
+    def test_do_or_die(self, confirm_mock):
         confirmed = '37347373737'
-        prompt_mock.return_value = confirmed
+        confirm_mock.return_value = confirmed
         result = formatting.no_going_back(confirmed)
         self.assertTrue(result)
 
         # no_going_back should cast int's to str()
         confirmed = '4712309182309'
-        prompt_mock.return_value = confirmed
+        confirm_mock.return_value = confirmed
         result = formatting.no_going_back(int(confirmed))
         self.assertTrue(result)
 
         confirmed = None
-        prompt_mock.return_value = ''
+        confirm_mock.return_value = ''
         result = formatting.no_going_back(confirmed)
         self.assertFalse(result)
 
@@ -98,9 +69,19 @@ def test_confirmation(self, prompt_mock):
         res = formatting.confirm('Confirm?', default=False)
         self.assertFalse(res)
 
-        prompt_mock.return_value = ''
+        prompt_mock.return_value = 'Y'
         res = formatting.confirm('Confirm?', default=True)
         self.assertTrue(res)
+        prompt_mock.assert_called_with('Confirm? [Y/n]',
+                                       default='y',
+                                       show_default=False)
+
+        prompt_mock.return_value = 'N'
+        res = formatting.confirm('Confirm?', default=False)
+        self.assertFalse(res)
+        prompt_mock.assert_called_with('Confirm? [y/N]',
+                                       default='n',
+                                       show_default=False)
 
 
 class FormattedItemTests(testing.TestCase):
diff --git a/SoftLayer/tests/CLI/modules/config_tests.py b/SoftLayer/tests/CLI/modules/config_tests.py
index 630cfa4c..fe835799 100644
--- a/SoftLayer/tests/CLI/modules/config_tests.py
+++ b/SoftLayer/tests/CLI/modules/config_tests.py
@@ -48,7 +48,7 @@ def test_setup(self, input, getpass, confirm_mock):
         with tempfile.NamedTemporaryFile() as config_file:
             confirm_mock.return_value = True
             getpass.return_value = 'A' * 64
-            input.side_effect = ['user', 'public']
+            input.side_effect = ['user', 'public', 0]
 
             result = self.run_command(['--config=%s' % config_file.name,
                                        'config', 'setup'])
@@ -71,7 +71,7 @@ def test_setup_cancel(self, input, getpass, confirm_mock):
         with tempfile.NamedTemporaryFile() as config_file:
             confirm_mock.return_value = False
             getpass.return_value = 'A' * 64
-            input.side_effect = ['user', 'public']
+            input.side_effect = ['user', 'public', 0]
 
             result = self.run_command(['--config=%s' % config_file.name,
                                        'config', 'setup'])
@@ -83,7 +83,7 @@ def test_setup_cancel(self, input, getpass, confirm_mock):
     @mock.patch('SoftLayer.CLI.environment.Environment.input')
     def test_get_user_input_private(self, input, getpass):
         getpass.return_value = 'A' * 64
-        input.side_effect = ['user', 'private']
+        input.side_effect = ['user', 'private', 0]
 
         username, secret, endpoint_url, timeout = (
             config.get_user_input(self.env))
@@ -96,7 +96,7 @@ def test_get_user_input_private(self, input, getpass):
     @mock.patch('SoftLayer.CLI.environment.Environment.input')
     def test_get_user_input_custom(self, input, getpass):
         getpass.return_value = 'A' * 64
-        input.side_effect = ['user', 'custom', 'custom-endpoint']
+        input.side_effect = ['user', 'custom', 'custom-endpoint', 0]
 
         _, _, endpoint_url, _ = config.get_user_input(self.env)
 
@@ -106,7 +106,7 @@ def test_get_user_input_custom(self, input, getpass):
     @mock.patch('SoftLayer.CLI.environment.Environment.input')
     def test_get_user_input_default(self, input, getpass):
         self.env.getpass.return_value = 'A' * 64
-        self.env.input.side_effect = ['user', '']
+        self.env.input.side_effect = ['user', 'public', 0]
 
         _, _, endpoint_url, _ = config.get_user_input(self.env)