From 7c9e5495f1dd09eae2dc478d2f447ef721a6b61d Mon Sep 17 00:00:00 2001 From: Lena Garber <114949949+lgarber-akamai@users.noreply.github.com> Date: Wed, 14 Jun 2023 12:33:31 -0400 Subject: [PATCH] new: Add a `No Default` option when running the `configure` command (#472) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 📝 Description This change adds a `No Default` option to the `linode-cli configure` command that allows users to optionally clear a default if it is already defined in the existing config. This differs from the skip (enter) functionality in that the skip functionality retains the previous config value and the `No Default` option drops the config value. ## ✔️ How to Test `make testunit` --- linodecli/configuration/__init__.py | 28 +++++- linodecli/configuration/helpers.py | 73 ++++++++++---- tests/unit/test_configuration.py | 150 ++++++++++++++++++++++++++++ 3 files changed, 230 insertions(+), 21 deletions(-) diff --git a/linodecli/configuration/__init__.py b/linodecli/configuration/__init__.py index 81360ce32..ad6f44d51 100644 --- a/linodecli/configuration/__init__.py +++ b/linodecli/configuration/__init__.py @@ -15,6 +15,7 @@ ) from .helpers import ( _check_browsers, + _config_get_with_default, _default_thing_input, _get_config, _get_config_path, @@ -382,6 +383,9 @@ def configure( regions, "Default Region (Optional): ", "Please select a valid Region, or press Enter to skip", + current_value=_config_get_with_default( + self.config, username, "region" + ), ) config["type"] = _default_thing_input( @@ -389,6 +393,9 @@ def configure( types, "Default Type of Linode (Optional): ", "Please select a valid Type, or press Enter to skip", + current_value=_config_get_with_default( + self.config, username, "type" + ), ) config["image"] = _default_thing_input( @@ -396,6 +403,9 @@ def configure( images, "Default Image (Optional): ", "Please select a valid Image, or press Enter to skip", + current_value=_config_get_with_default( + self.config, username, "image" + ), ) config["mysql_engine"] = _default_thing_input( @@ -403,6 +413,9 @@ def configure( mysql_engines, "Default Engine (Optional): ", "Please select a valid MySQL Database Engine, or press Enter to skip", + current_value=_config_get_with_default( + self.config, username, "mysql_engine" + ), ) config["postgresql_engine"] = _default_thing_input( @@ -410,6 +423,9 @@ def configure( postgresql_engines, "Default Engine (Optional): ", "Please select a valid PostgreSQL Database Engine, or press Enter to skip", + current_value=_config_get_with_default( + self.config, username, "postgresql_engine" + ), ) if auth_users: @@ -418,6 +434,9 @@ def configure( auth_users, "Default Option (Optional): ", "Please select a valid Option, or press Enter to skip", + current_value=_config_get_with_default( + self.config, username, "authorized_users" + ), ) # save off the new configuration @@ -446,8 +465,13 @@ def configure( print(f"Active user is now {username}") for k, v in config.items(): - if v: - self.config.set(username, k, v) + if v is None: + if self.config.has_option(username, k): + self.config.remove_option(username, k) + + continue + + self.config.set(username, k, v) self.write_config() os.chmod(_get_config_path(), 0o600) diff --git a/linodecli/configuration/helpers.py b/linodecli/configuration/helpers.py index 96db226cd..71abc72f4 100644 --- a/linodecli/configuration/helpers.py +++ b/linodecli/configuration/helpers.py @@ -5,6 +5,7 @@ import configparser import os import webbrowser +from typing import Any, Optional from .auth import _do_get_request @@ -84,7 +85,7 @@ def _check_browsers(): def _default_thing_input( - ask, things, prompt, error, optional=True + ask, things, prompt, error, optional=True, current_value=None ): # pylint: disable=too-many-arguments """ Requests the user choose from a list of things with the given prompt and @@ -92,30 +93,64 @@ def _default_thing_input( enter to not configure this option. """ print(f"\n{ask} Choices are:") + + exists = current_value is not None + + idx_offset = int(exists) + 1 + + # If there is a current value, users should have the option to clear it + if exists: + print(" 1 - No Default") + for ind, thing in enumerate(things): - print(f" {ind + 1} - {thing}") + print(f" {ind + idx_offset} - {thing}") print() - ret = "" while True: - choice = input(prompt) - - if choice: - try: - choice = int(choice) - choice = things[choice - 1] - except: - pass - - if choice in list(things): - ret = choice - break - print(error) - else: + choice_idx = input(prompt) + + if not choice_idx: + # The user wants to skip this config option if optional: - break + return current_value + + print(error) + continue + + try: + choice_idx = int(choice_idx) + except: + # Re-prompt if invalid value + continue + + # The user wants to drop this default + if exists and choice_idx == 1: + return None + + # We need to shift the index to account for the "No Default" option + choice_idx -= idx_offset + + # Validate index + if choice_idx >= len(things) or choice_idx < 0: print(error) - return ret + continue + + # Choice was valid; return + return things[choice_idx] + + +def _config_get_with_default( + config: configparser.ConfigParser, + user: str, + field: str, + default: Any = None, +) -> Optional[Any]: + """ + Gets a ConfigParser value and returns a default value if the key isn't found. + """ + return ( + config.get(user, field) if config.has_option(user, field) else default + ) def _handle_no_default_user(self): # pylint: disable=too-many-branches diff --git a/tests/unit/test_configuration.py b/tests/unit/test_configuration.py index 62ae29ff3..49a35d402 100644 --- a/tests/unit/test_configuration.py +++ b/tests/unit/test_configuration.py @@ -13,6 +13,7 @@ import requests_mock from linodecli import configuration +from linodecli.configuration import _default_thing_input class TestConfiguration: @@ -371,3 +372,152 @@ def mock_input(prompt): # make sure that we set the default engine value according to type of database assert conf.get_value("mysql_engine") == "mysql/test-engine" assert conf.get_value("postgresql_engine") == "postgresql/test-engine" + + def test_default_thing_input_no_current(self, monkeypatch): + stdout_buf = io.StringIO() + monkeypatch.setattr("sys.stdin", io.StringIO("1\n")) + + with contextlib.redirect_stdout(stdout_buf): + result = _default_thing_input( + "foo\n", ["foo", "bar"], "prompt text", "error text" + ) + + output_lines = stdout_buf.getvalue().splitlines() + + assert output_lines == [ + "", + "foo", + " Choices are:", + " 1 - foo", + " 2 - bar", + "", + "prompt text", + ] + + assert result == "foo" + + def test_default_thing_input_skip(self, monkeypatch): + stdout_buf = io.StringIO() + monkeypatch.setattr("sys.stdin", io.StringIO("\n")) + + with contextlib.redirect_stdout(stdout_buf): + result = _default_thing_input( + "foo\n", + ["foo", "bar"], + "prompt text", + "error text", + current_value="foo", + ) + + output_lines = stdout_buf.getvalue().splitlines() + + print(output_lines) + + assert output_lines == [ + "", + "foo", + " Choices are:", + " 1 - No Default", + " 2 - foo", + " 3 - bar", + "", + "prompt text", + ] + + assert result == "foo" + + def test_default_thing_input_no_default(self, monkeypatch): + stdout_buf = io.StringIO() + monkeypatch.setattr("sys.stdin", io.StringIO("1\n")) + + with contextlib.redirect_stdout(stdout_buf): + result = _default_thing_input( + "foo\n", + ["foo", "bar"], + "prompt text", + "error text", + current_value="foo", + ) + + output_lines = stdout_buf.getvalue().splitlines() + + print(output_lines) + + assert output_lines == [ + "", + "foo", + " Choices are:", + " 1 - No Default", + " 2 - foo", + " 3 - bar", + "", + "prompt text", + ] + + assert result is None + + def test_default_thing_input_valid(self, monkeypatch): + stdout_buf = io.StringIO() + monkeypatch.setattr("sys.stdin", io.StringIO("3\n")) + + with contextlib.redirect_stdout(stdout_buf): + result = _default_thing_input( + "foo\n", + ["foo", "bar"], + "prompt text", + "error text", + current_value="foo", + ) + + output_lines = stdout_buf.getvalue().splitlines() + + print(output_lines) + + assert output_lines == [ + "", + "foo", + " Choices are:", + " 1 - No Default", + " 2 - foo", + " 3 - bar", + "", + "prompt text", + ] + + assert result == "bar" + + def test_default_thing_input_valid_no_current(self, monkeypatch): + stdout_buf = io.StringIO() + monkeypatch.setattr("sys.stdin", io.StringIO("3\n1\n")) + + with contextlib.redirect_stdout(stdout_buf): + result = _default_thing_input( + "foo\n", + ["foo", "bar"], + "prompt text", + "error text", + ) + + output = stdout_buf.getvalue() + + assert "error text" in output + + assert result == "foo" + + def test_default_thing_input_out_of_range(self, monkeypatch): + stdout_buf = io.StringIO() + monkeypatch.setattr("sys.stdin", io.StringIO("4\n2\n")) + + with contextlib.redirect_stdout(stdout_buf): + result = _default_thing_input( + "foo\n", + ["foo", "bar"], + "prompt text", + "error text", + current_value="foo", + ) + + output = stdout_buf.getvalue() + assert "error text" in output + + assert result == "foo"