From 04003df62cfe264b6481aa3e61295075e380e0bf Mon Sep 17 00:00:00 2001 From: Maria Fernanda Magallanes Zubillaga Date: Tue, 20 Aug 2024 16:02:41 -0500 Subject: [PATCH] fix: implement feedback --- .../commands/enable_private_packages.py | 56 +++++++--------- tutorpicasso/commands/enable_themes.py | 35 +++++----- tutorpicasso/commands/run_extra_commands.py | 66 ++++++++++++------- 3 files changed, 82 insertions(+), 75 deletions(-) diff --git a/tutorpicasso/commands/enable_private_packages.py b/tutorpicasso/commands/enable_private_packages.py index 8ffdc68..736eba7 100644 --- a/tutorpicasso/commands/enable_private_packages.py +++ b/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 diff --git a/tutorpicasso/commands/enable_themes.py b/tutorpicasso/commands/enable_themes.py index 176608b..cc15dfe 100644 --- a/tutorpicasso/commands/enable_themes.py +++ b/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], + ) diff --git a/tutorpicasso/commands/run_extra_commands.py b/tutorpicasso/commands/run_extra_commands.py index 1d48862..272224d 100644 --- a/tutorpicasso/commands/run_extra_commands.py +++ b/tutorpicasso/commands/run_extra_commands.py @@ -15,16 +15,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: @@ -49,17 +52,24 @@ def validate_commands(commands: Any) -> None: 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: @@ -99,7 +109,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 +126,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 +153,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)