fix: implement feedback

tutorpicasso/commands/enable_private_packages.py

@@ -17,7 +17,7 @@ def enable_private_packages() -> None:
     defining them as private.
 
     Raises:
-        Exception: If an error occurs during the cloning or defining process.
+        Exception: If there is not enough information to clone the repo.
     """
     context = click.get_current_context().obj
     tutor_root = context.root
@@ -41,39 +41,33 @@ def enable_private_packages() -> None:
             file.write("")
 
     for package, info in packages.items():
-        try:
-            if not {"name", "repo", "version"}.issubset(info):
-                raise KeyError(
-                    f"{package} is missing one of the required keys: 'name', 'repo', 'version'"
-                )
-
-            if os.path.isdir(f'{private_requirements_root}/{info["name"]}'):
-                subprocess.call(
-                    ["rm", "-rf", f'{private_requirements_root}/{info["name"]}']
-                )
+        if not {"name", "repo", "version"}.issubset(info):
+            raise click.ClickException(
+                f"{package} is missing one of the required keys: 'name', 'repo', 'version'"
+            )
 
+        if os.path.isdir(f'{private_requirements_root}/{info["name"]}'):
             subprocess.call(
-                ["git", "clone", "-b", info["version"], info["repo"]],
-                cwd=private_requirements_root,
+                ["rm", "-rf", f'{private_requirements_root}/{info["name"]}']
             )
 
-            if tutor_version_obj < quince_version_obj:
-                echo_command = (
-                    f'echo "-e ./{info["name"]}/" >> {private_requirements_txt}'
-                )
-                subprocess.call(echo_command, shell=True)
-            else:
-                subprocess.call(
-                    [
-                        "tutor",
-                        "mounts",
-                        "add",
-                        f'{private_requirements_root}/{info["name"]}',
-                    ]
-                )
+        subprocess.call(
+            ["git", "clone", "-b", info["version"], info["repo"]],
+            cwd=private_requirements_root,
+        )
 
-        except KeyError as e:
-            raise click.ClickException(str(e))
+        if tutor_version_obj < quince_version_obj:
+            echo_command = f'echo "-e ./{info["name"]}/" >> {private_requirements_txt}'
+            subprocess.call(echo_command, shell=True)
+        else:
+            subprocess.call(
+                [
+                    "tutor",
+                    "mounts",
+                    "add",
+                    f'{private_requirements_root}/{info["name"]}',
+                ]
+            )
 
 
 def get_picasso_packages(settings: Dict[str, Any]) -> Dict[str, Dict[str, Any]]:
@@ -88,8 +82,6 @@ def get_picasso_packages(settings: Dict[str, Any]) -> Dict[str, Dict[str, Any]]:
         and the values are package details.
     """
     picasso_packages = {
-        key: val
-        for key, val in settings.items()
-        if key.endswith("_DPKG") and val != "None"
+        key: val for key, val in settings.items() if key.endswith("_DPKG") and val
     }
     return picasso_packages

tutorpicasso/commands/enable_themes.py

@@ -18,21 +18,20 @@ def enable_themes() -> None:
     tutor_root = context.root
     tutor_conf = tutor_config.load(tutor_root)
 
-    if tutor_conf.get("PICASSO_THEMES"):
-        for theme in tutor_conf["PICASSO_THEMES"]:
-            try:
-                if not {"name", "repo", "version"}.issubset(theme.keys()):
-                    raise KeyError(
-                        f"{theme} is missing one or more required keys: "
-                        "'name', 'repo', 'version'"
-                    )
-
-                theme_path = f'{tutor_root}/env/build/openedx/themes/{theme["name"]}'
-                if os.path.isdir(theme_path):
-                    subprocess.call(["rm", "-rf", theme_path])
-
-                subprocess.call(
-                    ["git", "clone", "-b", theme["version"], theme["repo"], theme_path],
-                )
-            except KeyError as e:
-                raise click.ClickException(f"Error: {str(e)}")
+    if not tutor_conf.get("PICASSO_THEMES"):
+        return
+
+    for theme in tutor_conf["PICASSO_THEMES"]:
+        if not {"name", "repo", "version"}.issubset(theme.keys()):
+            raise click.ClickException(
+                f"{theme} is missing one or more required keys: "
+                "'name', 'repo', 'version'"
+            )
+
+        theme_path = f'{tutor_root}/env/build/openedx/themes/{theme["name"]}'
+        if os.path.isdir(theme_path):
+            subprocess.call(["rm", "-rf", theme_path])
+
+        subprocess.call(
+            ["git", "clone", "-b", theme["version"], theme["repo"], theme_path],
+        )

tutorpicasso/commands/run_extra_commands.py

@@ -1,5 +1,6 @@
 import re
 import subprocess
+from itertools import chain
 
 # Was necessary to use this for compatibility with Python 3.8
 from typing import Any, List
@@ -15,16 +16,19 @@ def run_extra_commands() -> None:
     """
     This command runs tutor commands defined in PICASSO_EXTRA_COMMANDS
     """
-    tutor_root = (
-        subprocess.check_output("tutor config printroot", shell=True)
-        .decode("utf-8")
-        .strip()
-    )
-    config: Any = tutor_config.load(tutor_root)
-    picasso_extra_commands: Any = config.get("PICASSO_EXTRA_COMMANDS", None)
-    if picasso_extra_commands is not None:
-        validate_commands(picasso_extra_commands)
-        list(map(run_command, picasso_extra_commands))
+    context = click.get_current_context().obj
+    tutor_conf = tutor_config.load(context.root)
+
+    picasso_extra_commands: Any = tutor_conf.get("PICASSO_EXTRA_COMMANDS", None)
+
+    if not picasso_extra_commands:
+        return
+
+    error_message = validate_commands(picasso_extra_commands)
+    if error_message:
+        raise click.ClickException(error_message)
+
+    list(map(run_command, picasso_extra_commands))
 
 
 def validate_commands(commands: Any) -> None:
@@ -38,28 +42,35 @@ def validate_commands(commands: Any) -> None:
     splitted_commands = [
         split_string(command, COMMAND_CHAINING_OPERATORS) for command in commands
     ]
-    flat_commands_array: List[str] = sum(splitted_commands, [])
+    flat_commands_list: List[str] = chain.from_iterable(splitted_commands)
 
     invalid_commands = []
     misspelled_commands = []
-    for command in flat_commands_array:
+    for command in flat_commands_list:
         if "tutor" not in command.lower():
             if find_tutor_misspelled(command):
                 misspelled_commands.append(command)
             else:
                 invalid_commands.append(command)
 
-        if invalid_commands or misspelled_commands:
-            error_message = (
+        error_message = ""
+
+        if invalid_commands:
+            error_message += (
                 f"Found some issues with the commands:\n\n"
-                f"{'=> Invalid commands: ' if invalid_commands else ''}"
-                f"{', '.join(invalid_commands) if invalid_commands else ''}\n"
-                f"{'=> Misspelled commands: ' if misspelled_commands else ''}"
-                f"{', '.join(misspelled_commands) if misspelled_commands else ''}\n"
-                f"Take a look at the official Tutor commands: "
-                f"https://docs.tutor.edly.io/reference/cli/index.html"
+                f"=> Invalid commands: {', '.join(invalid_commands)}\n"
+            )
+
+        if misspelled_commands:
+            error_message += (
+                f"=> Misspelled commands: {', '.join(misspelled_commands)}\n"
+            )
+
+        if error_message:
+            error_message += (
+                "Take a look at the official Tutor commands: "
+                "https://docs.tutor.edly.io/reference/cli/index.html"
             )
-            raise click.ClickException(error_message)
 
 
 def run_command(command: str) -> None:
@@ -69,7 +80,7 @@ def run_command(command: str) -> None:
     This method runs the extra command provided.
 
     Args:
-            command (str): Tutor command.
+        command (str): Tutor command.
     """
     try:
         with subprocess.Popen(
@@ -82,15 +93,13 @@ def run_command(command: str) -> None:
             text=True,
         ) as process:
 
-            # It is sent a 'y' to say 'yes' on overriding the existing folders
-            stdout, stderr = process.communicate(input="y")
+            stdout, stderr = process.communicate()
 
             if process.returncode != 0 or "error" in stderr.lower():
                 raise subprocess.CalledProcessError(
                     process.returncode, command, output=stdout, stderr=stderr
                 )
 
-            # This print is left on purpose to show the command output
             click.echo(stdout)
 
     except subprocess.CalledProcessError as error:
@@ -99,7 +108,13 @@ def run_command(command: str) -> None:
 
 def find_tutor_misspelled(command: str) -> bool:
     """
-    This function takes a command and looks if it has the word 'tutor' misspelled
+    Look for misspelled occurrences of the word `tutor` in a given string. E.g. ...
+
+    Args:
+        command (str): string to be reviewed.
+
+    Return:
+        True if any misspelled occurrence is found, False otherwise.
 
     Args:
         command (str): Command to be reviewed
@@ -110,19 +125,19 @@ def find_tutor_misspelled(command: str) -> bool:
     return bool(re.match(r"[tT](?:[oru]{3}|[oru]{2}[rR]|[oru]u?)", command))
 
 
-def create_regex_from_array(arr: List[str]) -> re.Pattern[str]:
+def create_regex_from_list(special_chars: List[str]) -> re.Pattern[str]:
     """
     Compile a new regex and escape special characters in the given string.
     escaping special characters
 
     Args:
-        arr (list[str]): String that would be used to create a new regex
+        special_chars (list[str]): String that would be used to create a new regex
 
     Return:
         A new compiled regex pattern that can be used for comparisons
     """
-    escaped_arr = list(map(re.escape, arr))
-    regex_pattern = "|".join(escaped_arr)
+    escaped_special_chars = list(map(re.escape, special_chars))
+    regex_pattern = "|".join(escaped_special_chars)
     return re.compile(regex_pattern)
 
 
@@ -137,4 +152,4 @@ def split_string(string: str, split_by: List[str]) -> List[str]:
     Return:
         The string split into a list
     """
-    return re.split(create_regex_from_array(split_by), string)
+    return re.split(create_regex_from_list(split_by), string)