From 9eb1314f60902ecef59dd1cbd7ba525d542aef25 Mon Sep 17 00:00:00 2001 From: Guilherme Caponetto <638737+caponetto@users.noreply.github.com> Date: Tue, 13 Aug 2024 15:39:54 -0300 Subject: [PATCH 01/12] Add script to automate the creation of a new Python-based image from an existing one --- scripts/README.md | 29 ++ scripts/new_python_based_image.py | 555 ++++++++++++++++++++++++++++++ 2 files changed, 584 insertions(+) create mode 100644 scripts/new_python_based_image.py diff --git a/scripts/README.md b/scripts/README.md index aaead3450..2584ea64c 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -18,3 +18,32 @@ Update the `pandas` library to version `2.2.2` in all files under `./myproject` ./update_library_version.sh ./myproject pandas 2.2.2 'include|this' 'exclude|that' false ``` +## new_python_based_image.py + +This Python script generates a new folder structure for a Python-based project by copying an existing one and updating Python version references. It performs the following steps: + +1. Copy Folder: Duplicates the specified folder structure. +1. Update Python Versions: Replaces occurrences of the source Python version with the target version throughout the folder. +1. Update Lock Files: Executes `pipenv lock` to regenerate lock files based on the new Python version. + +If `pipenv lock` encounters errors, manual intervention is required to resolve compatibility issues between dependencies. Review the errors reported by `pipenv` and adjust the dependencies as needed. + +### Examples + +Create a Python 3.12 version based on the Python 3.9 version for each one that is in the `./base` directory: + +```sh +python scripts/new_python_based_image.py --context-dir . --source 3.9 --target 3.12 --match ./base +``` + +Create a Python 3.11 version based on the Python 3.9 versions for each one that is in the `./jupyter/rocm` directory: + +```sh +python scripts/new_python_based_image.py --context-dir . --source 3.9 --target 3.11 --match ./jupyter/rocm +``` + +Create a Python 3.11 version based on the Python 3.9 version for each one in the repository with `DEBUG` logs enabled: + +```sh +python scripts/new_python_based_image.py --context-dir . --source 3.9 --target 3.11 --match ./ --log-level DEBUG +``` diff --git a/scripts/new_python_based_image.py b/scripts/new_python_based_image.py new file mode 100644 index 000000000..152aca3dd --- /dev/null +++ b/scripts/new_python_based_image.py @@ -0,0 +1,555 @@ +import argparse +import os +import shutil +import sys +import platform +import subprocess +import logging +import re + + +LOGGER = logging.getLogger(__name__) + + +def configure_logger(log_level): + """ + Configures the logging settings based on the provided log level. + + Args: + log_level (str): The logging level to set (e.g., 'INFO', 'DEBUG'). + """ + level = getattr(logging, log_level.upper(), logging.INFO) + logging.basicConfig( + level=level, + format="[%(levelname)s] %(asctime)s: %(message)s", + datefmt="%H:%M:%S" + ) + LOGGER.setLevel(log_level) + + +class Args: + """ + Class to encapsulate command-line arguments. + + Attributes: + context_dir (str): The directory to use as the context for searching. + source (str): The source Python version. + target (str): The target Python version. + match (str): The string to match with the paths. + log_level (str): The logging level. + """ + + def __init__(self, context_dir, source, target, match, log_level): + self.context_dir = context_dir + self.source = source + self.target = target + self.match = match + self.log_level = log_level + + def __str__(self): + return (f"Arguments:\n" + f"Context Directory: {self.context_dir}\n" + f"Source Version: {self.source}\n" + f"Target Version: {self.target}\n" + f"Match: {self.match}\n" + f"Log Level: {self.log_level}") + + +def extract_input_args(): + """ + Extracts and validates command-line arguments. + + Returns: + Args: An instance of the Args class containing parsed arguments. + """ + parser = argparse.ArgumentParser( + description="Script to create a new Python-based image from an existing one.", + usage="python script.py --context-dir --source --target --match [--log-level ]" + ) + + parser.add_argument( + "--context-dir", + help="The directory to be the context for searching.") + parser.add_argument( + "--source", + help="The Python version to base the new image from.") + parser.add_argument( + "--target", + help="The Python version to be used in the new image.") + parser.add_argument( + "--match", + help="The string to match with the paths to base the new image from.") + parser.add_argument( + "--log-level", default="INFO", + help="Set the logging level. Default: INFO.") + + args = parser.parse_args() + + missing_args = [arg for arg, value in vars(args).items() if value is None] + if missing_args: + print(f"Missing required arguments: {', '.join(missing_args)}") + parser.print_help() + sys.exit(1) + + return Args(args.context_dir, args.source, args.target, args.match, args.log_level) + + +def extract_python_version(version): + """ + Extracts the major and minor version components from a Python version string. + + Args: + version (str): The Python version string (e.g., '3.9'). + + Returns: + list: A list containing the major and minor version components as strings. + """ + return version.split(".")[:2] + + +def check_python_version(version): + """ + Validates the format of a Python version string. + + Args: + version (str): The Python version string to validate. + + Exits the program with an error if the format is invalid. + """ + if not re.match(r'^\d+\.\d+$', version): + LOGGER.error( + f"Invalid Python version format: '{version}'. Expected format is . (e.g., '3.9').") + sys.exit(1) + + +def check_target_python_version_installed(version): + """ + Checks if the specified Python version is installed on the system. + + Args: + version (str): The Python version to check. + + Exits the program with an error if the version is not installed. + """ + python_executable = f"python{version}" + if shutil.which(python_executable) is None: + LOGGER.error(f"Python {version} is not installed.") + sys.exit(1) + + +def check_input_versions_not_equal(source_version, target_version): + """ + Ensures that the source and target Python versions are different. + + Args: + source_version (str): The source Python version. + target_version (str): The target Python version. + + Exits the program with an error if the versions are the same. + """ + if source_version == target_version: + LOGGER.error("Source and target Python versions must be different.") + sys.exit(1) + + +def check_os_linux(): + """ + Checks if the script is being run on a Linux operating system. + + Exits the program with an error if the OS is not Linux. + """ + LOGGER.debug(f"Operating system: {platform.system()}") + if platform.system() != "Linux": + LOGGER.error( + "This script can only be run on a Linux operating system.") + sys.exit(1) + + +def check_pipenv_installed(): + """ + Checks if `pipenv` is installed on the system. + + Exits the program with an error if `pipenv` is not found. + """ + if shutil.which("pipenv") is None: + LOGGER.error("pipenv is not installed.") + sys.exit(1) + + +def check_requirements(args): + """ + Performs various checks to ensure that all requirements are met. + + Args: + args (Args): An instance of the Args class containing the command-line arguments. + """ + check_os_linux() + check_python_version(args.source) + check_python_version(args.target) + check_input_versions_not_equal(args.source, args.target) + check_target_python_version_installed(args.target) + check_pipenv_installed() + + +def find_matching_paths(context_dir, source_version, match): + """ + Finds directories in the context directory that match the specified source version and match criteria. + + Args: + context_dir (str): The directory to search in. + source_version (str): The Python version to match. + match (str): The string to match with the paths. + + Returns: + list: A list of directories that match the criteria and contain a Dockerfile. + """ + blocklist = [os.path.join(".", path) for path in [".git", ".github", "ci"]] + matching_paths = [] + processed_dirs = set() + + for root, dirs, files in os.walk(context_dir): + if any(blocked in root for blocked in blocklist): + LOGGER.debug(f"Skipping '{root}' - blocked directory") + dirs[:] = [] + continue + + if source_version in root and match in root: + if "Dockerfile" in files and root not in processed_dirs: + LOGGER.debug(f"Found matching path with Dockerfile: '{root}'") + matching_paths.append(root) + processed_dirs.add(root) + else: + LOGGER.debug(f"Skipping match '{root}' - Dockerfile not found") + dirs[:] = [] + + return [p for p in matching_paths if source_version in p] + + +def replace_python_version_on_paths(paths_list, source_version, target_version): + """ + Replaces occurrences of the source Python version with the target version in a list of paths. + + Args: + paths_list (list): The list of paths to modify. + source_version (str): The source Python version. + target_version (str): The target Python version. + + Returns: + dict: A dictionary where keys are original paths and values are modified paths with the target version. + """ + return {path: path.replace(source_version, target_version) for path in paths_list} + + +def copy_paths(paths_dict): + """ + Copies directories from source paths to destination paths. + + Args: + paths_dict (dict): A dictionary where keys are source paths and values are destination paths. + + Returns: + tuple: A tuple containing two lists: success_paths and failed_paths. + """ + success_paths = [] + failed_paths = [] + + for src_path, dst_path in paths_dict.items(): + if os.path.exists(dst_path): + LOGGER.debug(f"Path '{dst_path}' already exists.") + failed_paths.append(dst_path) + else: + try: + os.makedirs(os.path.dirname(dst_path), exist_ok=True) + shutil.copytree(src_path, dst_path, dirs_exist_ok=True) + success_paths.append(dst_path) + LOGGER.debug(f"Path '{src_path}' copied to {dst_path}") + except Exception as e: + LOGGER.error( + f"Error copying '{src_path}' to '{dst_path}': {e}") + failed_paths.append(dst_path) + + return success_paths, failed_paths + + +def replace_python_version_in_file(file_path, source_version, target_version): + """ + Replaces occurrences of the source Python version with the target version in a file. + + Args: + file_path (str): The path to the file. + source_version (str): The source Python version. + target_version (str): The target Python version. + """ + LOGGER.debug(f"Replacing Python versions in '{file_path}'") + + try: + with open(file_path, "r") as file: + content = file.read() + + content = replace_python_version_in_content( + content, source_version, target_version) + + with open(file_path, "w") as file: + file.write(content) + except Exception as e: + LOGGER.debug(f"Error replacing Python versions in '{file_path}': {e}") + + +def replace_python_version_in_content(content, source_version, target_version): + """ + Replaces occurrences of the source Python version with the target version in a content string. + + Args: + content (str): The content to modify. + source_version (str): The source Python version. + target_version (str): The target Python version. + + Returns: + str: The modified content with the target Python version. + """ + source_major, source_minor = extract_python_version(source_version) + target_major, target_minor = extract_python_version(target_version) + + # Example: 3.9 -> 3.11 + result = content.replace(source_version, + target_version) + + # Example: 3-9 -> 3-11 + result = result.replace( + f"{source_major}-{source_minor}", + f"{target_major}-{target_minor}") + + # Example: python-39 -> python-311 + result = result.replace(f"python-{source_major}{source_minor}", + f"python-{target_major}{target_minor}") + + # Example: py39 -> py-311 + result = result.replace(f"py{source_major}{source_minor}", + f"py{target_major}{target_minor}") + + return result + + +def dict_to_str(dictionary, enumerate_lines=False): + """ + Converts a dictionary to a string representation. + + Args: + dictionary (dict): The dictionary to convert. + enumerate_lines (bool): Whether to enumerate lines in the output. + + Returns: + str: The string representation of the dictionary. + """ + if enumerate_lines: + return '\n'.join(f"{i + 1}. '{k}' -> '{v}'" for i, (k, v) in enumerate(dictionary.items())) + else: + return '\n'.join(f"'{k}' -> '{v}'" for k, v in dictionary.items()) + + +def list_to_str(lst, enumerate_lines=False): + """ + Converts a list to a string representation. + + Args: + lst (list): The list to convert. + enumerate_lines (bool): Whether to enumerate lines in the output. + + Returns: + str: The string representation of the list. + """ + if enumerate_lines: + return "\n".join(f"{i + 1}. {item}" for i, item in enumerate(lst)) + else: + return "\n".join(lst) + + +def with_logged_execution(fn, title): + """ + Executes a function and logs the start and end of the execution. + + Args: + fn (callable): The function to execute. + title (str): The title to log for the execution. + + Returns: + The result of the executed function. + """ + LOGGER.info(f"{title}...") + result = fn() + LOGGER.info(f"{title}... Done.") + return result + + +def replace_version_in_directory(directory_path, source_version, target_version): + """ + Replaces occurrences of the source Python version with the target version in file and directory names within a directory. + + Args: + directory_path (str): The path to the directory. + source_version (str): The source Python version. + target_version (str): The target Python version. + """ + LOGGER.debug(f"Replacing Python versions in '{directory_path}'") + + def rename_file_and_replace_python_version(path, filename, source_version, target_version, is_file=True): + old_path = os.path.join(path, filename) + new_filename = replace_python_version_in_content(filename, + source_version, + target_version) + new_path = os.path.join(path, new_filename) + + if old_path != new_path: + os.rename(old_path, new_path) + LOGGER.debug( + f"Renamed {'file' if is_file else 'directory'}: {old_path} -> {new_path}") + + if is_file: + replace_python_version_in_file(new_path, + source_version, + target_version) + + return new_path + + for root, dirs, files in os.walk(directory_path, topdown=False): + for file_name in files: + rename_file_and_replace_python_version(root, + file_name, + source_version, + target_version, + is_file=True) + + for dir_name in dirs: + rename_file_and_replace_python_version(root, + dir_name, + source_version, + target_version, + is_file=False) + + +def process_paths(copied_paths, source_version, target_version): + """ + Processes the list of copied paths by replacing Python versions and running pipenv lock on Pipfiles. + + Args: + copied_paths (list): The list of copied paths to process. + source_version (str): The source Python version. + target_version (str): The target Python version. + """ + if copied_paths.count == 0: + LOGGER.info("No paths to process.") + return + + for path in copied_paths: + if not os.path.exists(path): + LOGGER.warning(f"The path '{path}' does not exist.") + continue + + replace_version_in_directory(path, source_version, target_version) + process_pipfiles(path, target_version) + + +def process_pipfiles(path, target_version): + """ + Processes Pipfiles in a given path by running `pipenv lock` on them. + + Args: + path (str): The path to search for Pipfiles. + target_version (str): The target Python version to use with `pipenv lock`. + """ + for root, _, files in os.walk(path): + for file_name in files: + if file_name.startswith("Pipfile") and "lock" not in file_name: + pipfile_path = os.path.join(root, file_name) + run_pipenv_lock(pipfile_path, target_version) + + +def run_pipenv_lock(pipfile_path, target_version): + """ + Runs `pipenv lock` for a specified Pipfile to generate a new lock file. + + Args: + pipfile_path (str): The path to the Pipfile. + target_version (str): The target Python version to use with `pipenv lock`. + """ + LOGGER.info(f"Running pipenv lock for '{pipfile_path}'") + env = os.environ.copy() + env["PIPENV_PIPFILE"] = os.path.basename(pipfile_path) + + try: + result = subprocess.run( + ["pipenv", "lock", "--python", target_version], + cwd=os.path.dirname(pipfile_path), + check=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + env=env + ) + LOGGER.debug(result.stdout.decode()) + except subprocess.CalledProcessError as e: + LOGGER.error(f"Error running pipenv lock for '{pipfile_path}'") + LOGGER.debug(e.stderr.decode()) + + +def manual_checks(): + """ + Provides a list of manual checks to perform after the script execution. + + Returns: + list: A list of manual checks to review. + """ + return [ + "Check the issues thrown during the script execution, if any, and fix them manually.", + "Check if the Python version replacements have been performed correctly on the new files.", + "Review and make the appropriate changes in the Makefile and CI-related files.", + "Push the changes to a new branch on your fork to build the new images with GitHub workflows.", + "Test the new images." + ] + + +def main(): + """ + Main function to execute the script. + """ + args = extract_input_args() + LOGGER.info(args) + + configure_logger(args.log_level) + + with_logged_execution(lambda: check_requirements(args), + "Checking requirements") + + paths = with_logged_execution(lambda: find_matching_paths(args.context_dir, args.source, args.match), + f"Finding matching paths with '{args.match}' and Python {args.source}") + + paths_dict = replace_python_version_on_paths(paths, + args.source, + args.target) + + if len(paths_dict) == 0: + LOGGER.info("No paths found to copy.") + return + + LOGGER.info( + f"New folders based on the input args:\n{dict_to_str(paths_dict, enumerate_lines=True)}") + + success_paths, failed_paths = with_logged_execution(lambda: copy_paths(paths_dict), + f"Trying to copy {len(paths_dict)} folders") + + LOGGER.info( + f"{len(success_paths)} folders have been copied successfully whereas {len(failed_paths)} failed.") + + if len(success_paths) > 0: + with_logged_execution(lambda: process_paths(success_paths, args.source, args.target), + "Processing copied folders") + + if len(failed_paths) > 0: + LOGGER.warning( + f"Failed to copy the following paths:\n{list_to_str(failed_paths)}") + + LOGGER.info( + f"Manual checks to perform after the script execution:\n{list_to_str(manual_checks(), enumerate_lines=True)}") + + +if __name__ == "__main__": + main() From fc82f17516f8a65d409387954db04ce3aabcec44 Mon Sep 17 00:00:00 2001 From: Guilherme Caponetto <638737+caponetto@users.noreply.github.com> Date: Wed, 14 Aug 2024 07:06:44 -0300 Subject: [PATCH 02/12] Add type annotations to function args --- scripts/new_python_based_image.py | 120 +++++++++++++++--------------- 1 file changed, 60 insertions(+), 60 deletions(-) diff --git a/scripts/new_python_based_image.py b/scripts/new_python_based_image.py index 152aca3dd..e20d6c0c9 100644 --- a/scripts/new_python_based_image.py +++ b/scripts/new_python_based_image.py @@ -11,12 +11,12 @@ LOGGER = logging.getLogger(__name__) -def configure_logger(log_level): +def configure_logger(log_level: str): """ Configures the logging settings based on the provided log level. Args: - log_level (str): The logging level to set (e.g., 'INFO', 'DEBUG'). + log_level: The logging level to set (e.g., 'INFO', 'DEBUG'). """ level = getattr(logging, log_level.upper(), logging.INFO) logging.basicConfig( @@ -32,14 +32,14 @@ class Args: Class to encapsulate command-line arguments. Attributes: - context_dir (str): The directory to use as the context for searching. - source (str): The source Python version. - target (str): The target Python version. - match (str): The string to match with the paths. - log_level (str): The logging level. + context_dir: The directory to use as the context for searching. + source: The source Python version. + target: The target Python version. + match: The string to match with the paths. + log_level: The logging level. """ - def __init__(self, context_dir, source, target, match, log_level): + def __init__(self, context_dir: str, source: str, target: str, match: str, log_level: str): self.context_dir = context_dir self.source = source self.target = target @@ -94,12 +94,12 @@ def extract_input_args(): return Args(args.context_dir, args.source, args.target, args.match, args.log_level) -def extract_python_version(version): +def extract_python_version(version: str): """ Extracts the major and minor version components from a Python version string. Args: - version (str): The Python version string (e.g., '3.9'). + version: The Python version string (e.g., '3.9'). Returns: list: A list containing the major and minor version components as strings. @@ -107,12 +107,12 @@ def extract_python_version(version): return version.split(".")[:2] -def check_python_version(version): +def check_python_version(version: str): """ Validates the format of a Python version string. Args: - version (str): The Python version string to validate. + version: The Python version string to validate. Exits the program with an error if the format is invalid. """ @@ -122,12 +122,12 @@ def check_python_version(version): sys.exit(1) -def check_target_python_version_installed(version): +def check_target_python_version_installed(version: str): """ Checks if the specified Python version is installed on the system. Args: - version (str): The Python version to check. + version: The Python version to check. Exits the program with an error if the version is not installed. """ @@ -137,13 +137,13 @@ def check_target_python_version_installed(version): sys.exit(1) -def check_input_versions_not_equal(source_version, target_version): +def check_input_versions_not_equal(source_version: str, target_version: str): """ Ensures that the source and target Python versions are different. Args: - source_version (str): The source Python version. - target_version (str): The target Python version. + source_version: The source Python version. + target_version: The target Python version. Exits the program with an error if the versions are the same. """ @@ -176,12 +176,12 @@ def check_pipenv_installed(): sys.exit(1) -def check_requirements(args): +def check_requirements(args: Args): """ Performs various checks to ensure that all requirements are met. Args: - args (Args): An instance of the Args class containing the command-line arguments. + args: An instance of the Args class containing the command-line arguments. """ check_os_linux() check_python_version(args.source) @@ -191,14 +191,14 @@ def check_requirements(args): check_pipenv_installed() -def find_matching_paths(context_dir, source_version, match): +def find_matching_paths(context_dir: str, source_version: str, match: str): """ Finds directories in the context directory that match the specified source version and match criteria. Args: - context_dir (str): The directory to search in. - source_version (str): The Python version to match. - match (str): The string to match with the paths. + context_dir: The directory to search in. + source_version: The Python version to match. + match: The string to match with the paths. Returns: list: A list of directories that match the criteria and contain a Dockerfile. @@ -225,14 +225,14 @@ def find_matching_paths(context_dir, source_version, match): return [p for p in matching_paths if source_version in p] -def replace_python_version_on_paths(paths_list, source_version, target_version): +def replace_python_version_on_paths(paths_list: list, source_version: str, target_version: str): """ Replaces occurrences of the source Python version with the target version in a list of paths. Args: - paths_list (list): The list of paths to modify. - source_version (str): The source Python version. - target_version (str): The target Python version. + paths_list: The list of paths to modify. + source_version: The source Python version. + target_version: The target Python version. Returns: dict: A dictionary where keys are original paths and values are modified paths with the target version. @@ -240,12 +240,12 @@ def replace_python_version_on_paths(paths_list, source_version, target_version): return {path: path.replace(source_version, target_version) for path in paths_list} -def copy_paths(paths_dict): +def copy_paths(paths_dict: dict): """ Copies directories from source paths to destination paths. Args: - paths_dict (dict): A dictionary where keys are source paths and values are destination paths. + paths_dict: A dictionary where keys are source paths and values are destination paths. Returns: tuple: A tuple containing two lists: success_paths and failed_paths. @@ -271,14 +271,14 @@ def copy_paths(paths_dict): return success_paths, failed_paths -def replace_python_version_in_file(file_path, source_version, target_version): +def replace_python_version_in_file(file_path: str, source_version: str, target_version: str): """ Replaces occurrences of the source Python version with the target version in a file. Args: - file_path (str): The path to the file. - source_version (str): The source Python version. - target_version (str): The target Python version. + file_path: The path to the file. + source_version: The source Python version. + target_version: The target Python version. """ LOGGER.debug(f"Replacing Python versions in '{file_path}'") @@ -295,14 +295,14 @@ def replace_python_version_in_file(file_path, source_version, target_version): LOGGER.debug(f"Error replacing Python versions in '{file_path}': {e}") -def replace_python_version_in_content(content, source_version, target_version): +def replace_python_version_in_content(content: str, source_version: str, target_version: str): """ Replaces occurrences of the source Python version with the target version in a content string. Args: - content (str): The content to modify. - source_version (str): The source Python version. - target_version (str): The target Python version. + content: The content to modify. + source_version: The source Python version. + target_version: The target Python version. Returns: str: The modified content with the target Python version. @@ -330,13 +330,13 @@ def replace_python_version_in_content(content, source_version, target_version): return result -def dict_to_str(dictionary, enumerate_lines=False): +def dict_to_str(dictionary: dict, enumerate_lines=False): """ Converts a dictionary to a string representation. Args: - dictionary (dict): The dictionary to convert. - enumerate_lines (bool): Whether to enumerate lines in the output. + dictionary: The dictionary to convert. + enumerate_lines: Whether to enumerate lines in the output. Returns: str: The string representation of the dictionary. @@ -347,13 +347,13 @@ def dict_to_str(dictionary, enumerate_lines=False): return '\n'.join(f"'{k}' -> '{v}'" for k, v in dictionary.items()) -def list_to_str(lst, enumerate_lines=False): +def list_to_str(lst: list, enumerate_lines=False): """ Converts a list to a string representation. Args: - lst (list): The list to convert. - enumerate_lines (bool): Whether to enumerate lines in the output. + lst: The list to convert. + enumerate_lines: Whether to enumerate lines in the output. Returns: str: The string representation of the list. @@ -364,13 +364,13 @@ def list_to_str(lst, enumerate_lines=False): return "\n".join(lst) -def with_logged_execution(fn, title): +def with_logged_execution(fn: callable, title: str): """ Executes a function and logs the start and end of the execution. Args: - fn (callable): The function to execute. - title (str): The title to log for the execution. + fn: The function to execute. + title: The title to log for the execution. Returns: The result of the executed function. @@ -381,14 +381,14 @@ def with_logged_execution(fn, title): return result -def replace_version_in_directory(directory_path, source_version, target_version): +def replace_version_in_directory(directory_path: str, source_version: str, target_version: str): """ Replaces occurrences of the source Python version with the target version in file and directory names within a directory. Args: - directory_path (str): The path to the directory. - source_version (str): The source Python version. - target_version (str): The target Python version. + directory_path: The path to the directory. + source_version: The source Python version. + target_version: The target Python version. """ LOGGER.debug(f"Replacing Python versions in '{directory_path}'") @@ -427,14 +427,14 @@ def rename_file_and_replace_python_version(path, filename, source_version, targe is_file=False) -def process_paths(copied_paths, source_version, target_version): +def process_paths(copied_paths: list, source_version: str, target_version: str): """ Processes the list of copied paths by replacing Python versions and running pipenv lock on Pipfiles. Args: - copied_paths (list): The list of copied paths to process. - source_version (str): The source Python version. - target_version (str): The target Python version. + copied_paths: The list of copied paths to process. + source_version: The source Python version. + target_version: The target Python version. """ if copied_paths.count == 0: LOGGER.info("No paths to process.") @@ -449,13 +449,13 @@ def process_paths(copied_paths, source_version, target_version): process_pipfiles(path, target_version) -def process_pipfiles(path, target_version): +def process_pipfiles(path: str, target_version: str): """ Processes Pipfiles in a given path by running `pipenv lock` on them. Args: - path (str): The path to search for Pipfiles. - target_version (str): The target Python version to use with `pipenv lock`. + path: The path to search for Pipfiles. + target_version: The target Python version to use with `pipenv lock`. """ for root, _, files in os.walk(path): for file_name in files: @@ -464,13 +464,13 @@ def process_pipfiles(path, target_version): run_pipenv_lock(pipfile_path, target_version) -def run_pipenv_lock(pipfile_path, target_version): +def run_pipenv_lock(pipfile_path: str, target_version: str): """ Runs `pipenv lock` for a specified Pipfile to generate a new lock file. Args: - pipfile_path (str): The path to the Pipfile. - target_version (str): The target Python version to use with `pipenv lock`. + pipfile_path: The path to the Pipfile. + target_version: The target Python version to use with `pipenv lock`. """ LOGGER.info(f"Running pipenv lock for '{pipfile_path}'") env = os.environ.copy() From 5017e7814a1bee93deae9f53377517e45bbcb529 Mon Sep 17 00:00:00 2001 From: Guilherme Caponetto <638737+caponetto@users.noreply.github.com> Date: Wed, 14 Aug 2024 07:07:17 -0300 Subject: [PATCH 03/12] Add missing folders to blocklist --- scripts/new_python_based_image.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/scripts/new_python_based_image.py b/scripts/new_python_based_image.py index e20d6c0c9..b2b86668c 100644 --- a/scripts/new_python_based_image.py +++ b/scripts/new_python_based_image.py @@ -203,7 +203,13 @@ def find_matching_paths(context_dir: str, source_version: str, match: str): Returns: list: A list of directories that match the criteria and contain a Dockerfile. """ - blocklist = [os.path.join(".", path) for path in [".git", ".github", "ci"]] + blocklist = [os.path.join(".", path) for path in [".git", + ".github", + "ci", + "docs", + "manifests", + "scripts", + "tests"]] matching_paths = [] processed_dirs = set() From 59a1bb13c124d37eca5536a3ef9010fa4bc477f5 Mon Sep 17 00:00:00 2001 From: Guilherme Caponetto <638737+caponetto@users.noreply.github.com> Date: Wed, 14 Aug 2024 07:11:13 -0300 Subject: [PATCH 04/12] Removing redundant comment --- scripts/new_python_based_image.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/scripts/new_python_based_image.py b/scripts/new_python_based_image.py index b2b86668c..b9886038d 100644 --- a/scripts/new_python_based_image.py +++ b/scripts/new_python_based_image.py @@ -514,9 +514,6 @@ def manual_checks(): def main(): - """ - Main function to execute the script. - """ args = extract_input_args() LOGGER.info(args) From 8303ea64f6061b58148198b21ace72e8b99c5ac2 Mon Sep 17 00:00:00 2001 From: Guilherme Caponetto <638737+caponetto@users.noreply.github.com> Date: Wed, 14 Aug 2024 07:20:33 -0300 Subject: [PATCH 05/12] Use dataclass in the Args class --- scripts/new_python_based_image.py | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/scripts/new_python_based_image.py b/scripts/new_python_based_image.py index b9886038d..b7c8f3c2b 100644 --- a/scripts/new_python_based_image.py +++ b/scripts/new_python_based_image.py @@ -6,6 +6,7 @@ import subprocess import logging import re +from dataclasses import dataclass LOGGER = logging.getLogger(__name__) @@ -27,24 +28,16 @@ def configure_logger(log_level: str): LOGGER.setLevel(log_level) +@dataclass class Args: """ Class to encapsulate command-line arguments. - - Attributes: - context_dir: The directory to use as the context for searching. - source: The source Python version. - target: The target Python version. - match: The string to match with the paths. - log_level: The logging level. """ - - def __init__(self, context_dir: str, source: str, target: str, match: str, log_level: str): - self.context_dir = context_dir - self.source = source - self.target = target - self.match = match - self.log_level = log_level + context_dir: str + source: str + target: str + match: str + log_level: str def __str__(self): return (f"Arguments:\n" From 353fa60c5ff84cd04d5d88e414a1ddd781020708 Mon Sep 17 00:00:00 2001 From: Guilherme Caponetto <638737+caponetto@users.noreply.github.com> Date: Wed, 14 Aug 2024 07:35:04 -0300 Subject: [PATCH 06/12] Use context manager for logged_execution --- scripts/new_python_based_image.py | 47 +++++++++++++++---------------- 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/scripts/new_python_based_image.py b/scripts/new_python_based_image.py index b7c8f3c2b..6e8cedd88 100644 --- a/scripts/new_python_based_image.py +++ b/scripts/new_python_based_image.py @@ -1,11 +1,12 @@ import argparse +import contextlib +import logging import os -import shutil -import sys import platform -import subprocess -import logging import re +import shutil +import subprocess +import sys from dataclasses import dataclass @@ -363,21 +364,17 @@ def list_to_str(lst: list, enumerate_lines=False): return "\n".join(lst) -def with_logged_execution(fn: callable, title: str): - """ - Executes a function and logs the start and end of the execution. - - Args: - fn: The function to execute. - title: The title to log for the execution. - - Returns: - The result of the executed function. +@contextlib.contextmanager +def logged_execution(title: str): + """Usage: + > with logged_execution("launching rockets"): + > ... result = launch_rockets() """ LOGGER.info(f"{title}...") - result = fn() - LOGGER.info(f"{title}... Done.") - return result + try: + yield None + finally: + LOGGER.info(f"{title}... Done.") def replace_version_in_directory(directory_path: str, source_version: str, target_version: str): @@ -512,11 +509,11 @@ def main(): configure_logger(args.log_level) - with_logged_execution(lambda: check_requirements(args), - "Checking requirements") + with logged_execution("Checking requirements"): + check_requirements(args) - paths = with_logged_execution(lambda: find_matching_paths(args.context_dir, args.source, args.match), - f"Finding matching paths with '{args.match}' and Python {args.source}") + with logged_execution(f"Finding matching paths with '{args.match}' and Python {args.source}"): + paths = find_matching_paths(args.context_dir, args.source, args.match) paths_dict = replace_python_version_on_paths(paths, args.source, @@ -529,15 +526,15 @@ def main(): LOGGER.info( f"New folders based on the input args:\n{dict_to_str(paths_dict, enumerate_lines=True)}") - success_paths, failed_paths = with_logged_execution(lambda: copy_paths(paths_dict), - f"Trying to copy {len(paths_dict)} folders") + with logged_execution(f"Trying to copy {len(paths_dict)} folders"): + success_paths, failed_paths = copy_paths(paths_dict) LOGGER.info( f"{len(success_paths)} folders have been copied successfully whereas {len(failed_paths)} failed.") if len(success_paths) > 0: - with_logged_execution(lambda: process_paths(success_paths, args.source, args.target), - "Processing copied folders") + with logged_execution("Processing copied folders"): + process_paths(success_paths, args.source, args.target) if len(failed_paths) > 0: LOGGER.warning( From 1a38673c328014ac3a00cb01cb23f94d86b4ff2a Mon Sep 17 00:00:00 2001 From: Guilherme Caponetto <638737+caponetto@users.noreply.github.com> Date: Wed, 14 Aug 2024 07:42:55 -0300 Subject: [PATCH 07/12] Set encoding prop on subprocess.run --- scripts/new_python_based_image.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/scripts/new_python_based_image.py b/scripts/new_python_based_image.py index 6e8cedd88..83724d285 100644 --- a/scripts/new_python_based_image.py +++ b/scripts/new_python_based_image.py @@ -479,12 +479,13 @@ def run_pipenv_lock(pipfile_path: str, target_version: str): check=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, - env=env + env=env, + encoding="utf-8" ) - LOGGER.debug(result.stdout.decode()) + LOGGER.debug(result.stdout) except subprocess.CalledProcessError as e: LOGGER.error(f"Error running pipenv lock for '{pipfile_path}'") - LOGGER.debug(e.stderr.decode()) + LOGGER.debug(e.stderr) def manual_checks(): From b6851b56d34ada1bcf6d0f681d9d3acff6c1bfa2 Mon Sep 17 00:00:00 2001 From: Guilherme Caponetto <638737+caponetto@users.noreply.github.com> Date: Wed, 14 Aug 2024 07:44:10 -0300 Subject: [PATCH 08/12] Improve log level help message --- scripts/new_python_based_image.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/new_python_based_image.py b/scripts/new_python_based_image.py index 83724d285..49c8a5e4d 100644 --- a/scripts/new_python_based_image.py +++ b/scripts/new_python_based_image.py @@ -75,7 +75,7 @@ def extract_input_args(): help="The string to match with the paths to base the new image from.") parser.add_argument( "--log-level", default="INFO", - help="Set the logging level. Default: INFO.") + help="Set the logging level. Default: %(default)s).") args = parser.parse_args() From e503827773cc6b002381cdc95c4bc71dd5b632f2 Mon Sep 17 00:00:00 2001 From: Guilherme Caponetto <638737+caponetto@users.noreply.github.com> Date: Wed, 14 Aug 2024 08:30:43 -0300 Subject: [PATCH 09/12] Return non-zero code for non-happy scenarios --- scripts/new_python_based_image.py | 49 ++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/scripts/new_python_based_image.py b/scripts/new_python_based_image.py index 49c8a5e4d..085ca3e4b 100644 --- a/scripts/new_python_based_image.py +++ b/scripts/new_python_based_image.py @@ -432,17 +432,15 @@ def process_paths(copied_paths: list, source_version: str, target_version: str): source_version: The source Python version. target_version: The target Python version. """ - if copied_paths.count == 0: - LOGGER.info("No paths to process.") - return - for path in copied_paths: if not os.path.exists(path): LOGGER.warning(f"The path '{path}' does not exist.") continue replace_version_in_directory(path, source_version, target_version) - process_pipfiles(path, target_version) + success_processed, failed_processed = process_pipfiles(path, + target_version) + return success_processed, failed_processed def process_pipfiles(path: str, target_version: str): @@ -453,11 +451,15 @@ def process_pipfiles(path: str, target_version: str): path: The path to search for Pipfiles. target_version: The target Python version to use with `pipenv lock`. """ + success_processed = [] + failed_processed = [] for root, _, files in os.walk(path): for file_name in files: if file_name.startswith("Pipfile") and "lock" not in file_name: pipfile_path = os.path.join(root, file_name) - run_pipenv_lock(pipfile_path, target_version) + result = run_pipenv_lock(pipfile_path, target_version) + (success_processed if result else failed_processed).append(pipfile_path) + return success_processed, failed_processed def run_pipenv_lock(pipfile_path: str, target_version: str): @@ -483,9 +485,10 @@ def run_pipenv_lock(pipfile_path: str, target_version: str): encoding="utf-8" ) LOGGER.debug(result.stdout) + return True except subprocess.CalledProcessError as e: - LOGGER.error(f"Error running pipenv lock for '{pipfile_path}'") LOGGER.debug(e.stderr) + return False def manual_checks(): @@ -522,28 +525,40 @@ def main(): if len(paths_dict) == 0: LOGGER.info("No paths found to copy.") - return + sys.exit(1) LOGGER.info( - f"New folders based on the input args:\n{dict_to_str(paths_dict, enumerate_lines=True)}") + f"New folder(s) based on the input args:\n{dict_to_str(paths_dict, enumerate_lines=True)}") - with logged_execution(f"Trying to copy {len(paths_dict)} folders"): - success_paths, failed_paths = copy_paths(paths_dict) + with logged_execution(f"Trying to copy {len(paths_dict)} folder(s)"): + success_copied_paths, failed_copied_paths = copy_paths(paths_dict) LOGGER.info( - f"{len(success_paths)} folders have been copied successfully whereas {len(failed_paths)} failed.") + f"{len(success_copied_paths)} folder(s) have been copied successfully whereas {len(failed_copied_paths)} failed.") - if len(success_paths) > 0: - with logged_execution("Processing copied folders"): - process_paths(success_paths, args.source, args.target) + exit_code = 0 - if len(failed_paths) > 0: + if len(failed_copied_paths) > 0: LOGGER.warning( - f"Failed to copy the following paths:\n{list_to_str(failed_paths)}") + f"Failed to copy the following folder(s):\n{list_to_str(failed_copied_paths, enumerate_lines=True)}") + exit_code = 1 + + if len(success_copied_paths) > 0: + with logged_execution("Processing copied folders"): + _, failed_processed = process_paths(success_copied_paths, + args.source, + args.target) + + if len(failed_processed) > 0: + LOGGER.warning( + f"Failed to process the following lock files:\n{list_to_str(failed_processed, enumerate_lines=True)}") + exit_code = 1 LOGGER.info( f"Manual checks to perform after the script execution:\n{list_to_str(manual_checks(), enumerate_lines=True)}") + sys.exit(exit_code) + if __name__ == "__main__": main() From bda4052011e82ad2733e77ac54b0c5c1dec58acc Mon Sep 17 00:00:00 2001 From: Guilherme Caponetto <638737+caponetto@users.noreply.github.com> Date: Wed, 14 Aug 2024 08:57:28 -0300 Subject: [PATCH 10/12] Add type annotations to function return --- scripts/new_python_based_image.py | 68 ++++++++++++++++--------------- 1 file changed, 35 insertions(+), 33 deletions(-) diff --git a/scripts/new_python_based_image.py b/scripts/new_python_based_image.py index 085ca3e4b..a4f0d41d8 100644 --- a/scripts/new_python_based_image.py +++ b/scripts/new_python_based_image.py @@ -49,12 +49,12 @@ def __str__(self): f"Log Level: {self.log_level}") -def extract_input_args(): +def extract_input_args() -> Args: """ Extracts and validates command-line arguments. Returns: - Args: An instance of the Args class containing parsed arguments. + An instance of the Args class containing parsed arguments. """ parser = argparse.ArgumentParser( description="Script to create a new Python-based image from an existing one.", @@ -88,7 +88,7 @@ def extract_input_args(): return Args(args.context_dir, args.source, args.target, args.match, args.log_level) -def extract_python_version(version: str): +def extract_python_version(version: str) -> list: """ Extracts the major and minor version components from a Python version string. @@ -96,7 +96,7 @@ def extract_python_version(version: str): version: The Python version string (e.g., '3.9'). Returns: - list: A list containing the major and minor version components as strings. + A list containing the major and minor version components of a Python version. """ return version.split(".")[:2] @@ -107,8 +107,6 @@ def check_python_version(version: str): Args: version: The Python version string to validate. - - Exits the program with an error if the format is invalid. """ if not re.match(r'^\d+\.\d+$', version): LOGGER.error( @@ -122,8 +120,6 @@ def check_target_python_version_installed(version: str): Args: version: The Python version to check. - - Exits the program with an error if the version is not installed. """ python_executable = f"python{version}" if shutil.which(python_executable) is None: @@ -138,8 +134,6 @@ def check_input_versions_not_equal(source_version: str, target_version: str): Args: source_version: The source Python version. target_version: The target Python version. - - Exits the program with an error if the versions are the same. """ if source_version == target_version: LOGGER.error("Source and target Python versions must be different.") @@ -149,8 +143,6 @@ def check_input_versions_not_equal(source_version: str, target_version: str): def check_os_linux(): """ Checks if the script is being run on a Linux operating system. - - Exits the program with an error if the OS is not Linux. """ LOGGER.debug(f"Operating system: {platform.system()}") if platform.system() != "Linux": @@ -162,8 +154,6 @@ def check_os_linux(): def check_pipenv_installed(): """ Checks if `pipenv` is installed on the system. - - Exits the program with an error if `pipenv` is not found. """ if shutil.which("pipenv") is None: LOGGER.error("pipenv is not installed.") @@ -185,7 +175,7 @@ def check_requirements(args: Args): check_pipenv_installed() -def find_matching_paths(context_dir: str, source_version: str, match: str): +def find_matching_paths(context_dir: str, source_version: str, match: str) -> list: """ Finds directories in the context directory that match the specified source version and match criteria. @@ -195,7 +185,7 @@ def find_matching_paths(context_dir: str, source_version: str, match: str): match: The string to match with the paths. Returns: - list: A list of directories that match the criteria and contain a Dockerfile. + A list of directories that match the criteria and contain a Dockerfile. """ blocklist = [os.path.join(".", path) for path in [".git", ".github", @@ -225,7 +215,7 @@ def find_matching_paths(context_dir: str, source_version: str, match: str): return [p for p in matching_paths if source_version in p] -def replace_python_version_on_paths(paths_list: list, source_version: str, target_version: str): +def replace_python_version_on_paths(paths_list: list, source_version: str, target_version: str) -> dict: """ Replaces occurrences of the source Python version with the target version in a list of paths. @@ -235,12 +225,12 @@ def replace_python_version_on_paths(paths_list: list, source_version: str, targe target_version: The target Python version. Returns: - dict: A dictionary where keys are original paths and values are modified paths with the target version. + A dictionary where keys are original paths and values are modified paths with the target version. """ return {path: path.replace(source_version, target_version) for path in paths_list} -def copy_paths(paths_dict: dict): +def copy_paths(paths_dict: dict) -> tuple: """ Copies directories from source paths to destination paths. @@ -248,7 +238,7 @@ def copy_paths(paths_dict: dict): paths_dict: A dictionary where keys are source paths and values are destination paths. Returns: - tuple: A tuple containing two lists: success_paths and failed_paths. + A tuple of two lists for the successfully and failed copied paths. """ success_paths = [] failed_paths = [] @@ -295,7 +285,7 @@ def replace_python_version_in_file(file_path: str, source_version: str, target_v LOGGER.debug(f"Error replacing Python versions in '{file_path}': {e}") -def replace_python_version_in_content(content: str, source_version: str, target_version: str): +def replace_python_version_in_content(content: str, source_version: str, target_version: str) -> str: """ Replaces occurrences of the source Python version with the target version in a content string. @@ -305,7 +295,7 @@ def replace_python_version_in_content(content: str, source_version: str, target_ target_version: The target Python version. Returns: - str: The modified content with the target Python version. + The modified content with the target Python version. """ source_major, source_minor = extract_python_version(source_version) target_major, target_minor = extract_python_version(target_version) @@ -330,7 +320,7 @@ def replace_python_version_in_content(content: str, source_version: str, target_ return result -def dict_to_str(dictionary: dict, enumerate_lines=False): +def dict_to_str(dictionary: dict, enumerate_lines=False) -> str: """ Converts a dictionary to a string representation. @@ -339,7 +329,7 @@ def dict_to_str(dictionary: dict, enumerate_lines=False): enumerate_lines: Whether to enumerate lines in the output. Returns: - str: The string representation of the dictionary. + The string representation of the dictionary. """ if enumerate_lines: return '\n'.join(f"{i + 1}. '{k}' -> '{v}'" for i, (k, v) in enumerate(dictionary.items())) @@ -347,7 +337,7 @@ def dict_to_str(dictionary: dict, enumerate_lines=False): return '\n'.join(f"'{k}' -> '{v}'" for k, v in dictionary.items()) -def list_to_str(lst: list, enumerate_lines=False): +def list_to_str(lst: list, enumerate_lines=False) -> str: """ Converts a list to a string representation. @@ -356,7 +346,7 @@ def list_to_str(lst: list, enumerate_lines=False): enumerate_lines: Whether to enumerate lines in the output. Returns: - str: The string representation of the list. + The string representation of the list. """ if enumerate_lines: return "\n".join(f"{i + 1}. {item}" for i, item in enumerate(lst)) @@ -377,7 +367,7 @@ def logged_execution(title: str): LOGGER.info(f"{title}... Done.") -def replace_version_in_directory(directory_path: str, source_version: str, target_version: str): +def replace_version_in_directory(directory_path: str, source_version: str, target_version: str) -> str: """ Replaces occurrences of the source Python version with the target version in file and directory names within a directory. @@ -385,6 +375,9 @@ def replace_version_in_directory(directory_path: str, source_version: str, targe directory_path: The path to the directory. source_version: The source Python version. target_version: The target Python version. + + Returns: + The path to the renamed file or directory. """ LOGGER.debug(f"Replacing Python versions in '{directory_path}'") @@ -423,14 +416,17 @@ def rename_file_and_replace_python_version(path, filename, source_version, targe is_file=False) -def process_paths(copied_paths: list, source_version: str, target_version: str): +def process_paths(copied_paths: list, source_version: str, target_version: str) -> tuple: """ - Processes the list of copied paths by replacing Python versions and running pipenv lock on Pipfiles. + Processes the list of copied paths by replacing Python versions and running `pipenv lock` on Pipfiles. Args: copied_paths: The list of copied paths to process. source_version: The source Python version. target_version: The target Python version. + + Returns: + A tuple of two lists for the successfully and failed processed lock files. """ for path in copied_paths: if not os.path.exists(path): @@ -443,13 +439,16 @@ def process_paths(copied_paths: list, source_version: str, target_version: str): return success_processed, failed_processed -def process_pipfiles(path: str, target_version: str): +def process_pipfiles(path: str, target_version: str) -> tuple: """ Processes Pipfiles in a given path by running `pipenv lock` on them. Args: path: The path to search for Pipfiles. target_version: The target Python version to use with `pipenv lock`. + + Returns: + A tuple of two lists for the successfully and failed processed lock files. """ success_processed = [] failed_processed = [] @@ -462,13 +461,16 @@ def process_pipfiles(path: str, target_version: str): return success_processed, failed_processed -def run_pipenv_lock(pipfile_path: str, target_version: str): +def run_pipenv_lock(pipfile_path: str, target_version: str) -> bool: """ Runs `pipenv lock` for a specified Pipfile to generate a new lock file. Args: pipfile_path: The path to the Pipfile. target_version: The target Python version to use with `pipenv lock`. + + Returns: + Whether the `pipenv lock` command was successful or not. """ LOGGER.info(f"Running pipenv lock for '{pipfile_path}'") env = os.environ.copy() @@ -491,12 +493,12 @@ def run_pipenv_lock(pipfile_path: str, target_version: str): return False -def manual_checks(): +def manual_checks() -> list: """ Provides a list of manual checks to perform after the script execution. Returns: - list: A list of manual checks to review. + A list of manual checks to review. """ return [ "Check the issues thrown during the script execution, if any, and fix them manually.", From 6b4aafb4a370fae36a5713f33f2fa4d6395276cb Mon Sep 17 00:00:00 2001 From: Guilherme Caponetto <638737+caponetto@users.noreply.github.com> Date: Wed, 14 Aug 2024 13:42:39 -0300 Subject: [PATCH 11/12] Final adjustments --- scripts/new_python_based_image.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/scripts/new_python_based_image.py b/scripts/new_python_based_image.py index a4f0d41d8..04557840a 100644 --- a/scripts/new_python_based_image.py +++ b/scripts/new_python_based_image.py @@ -45,7 +45,7 @@ def __str__(self): f"Context Directory: {self.context_dir}\n" f"Source Version: {self.source}\n" f"Target Version: {self.target}\n" - f"Match: {self.match}\n" + f"Match: '{self.match}'\n" f"Log Level: {self.log_level}") @@ -349,7 +349,7 @@ def list_to_str(lst: list, enumerate_lines=False) -> str: The string representation of the list. """ if enumerate_lines: - return "\n".join(f"{i + 1}. {item}" for i, item in enumerate(lst)) + return "\n".join(f"{i + 1}. '{item}'" for i, item in enumerate(lst)) else: return "\n".join(lst) @@ -428,14 +428,17 @@ def process_paths(copied_paths: list, source_version: str, target_version: str) Returns: A tuple of two lists for the successfully and failed processed lock files. """ + success_processed = [] + failed_processed = [] for path in copied_paths: if not os.path.exists(path): LOGGER.warning(f"The path '{path}' does not exist.") continue replace_version_in_directory(path, source_version, target_version) - success_processed, failed_processed = process_pipfiles(path, - target_version) + success, failed = process_pipfiles(path, target_version) + success_processed.extend(success) + failed_processed.extend(failed) return success_processed, failed_processed @@ -511,9 +514,8 @@ def manual_checks() -> list: def main(): args = extract_input_args() - LOGGER.info(args) - configure_logger(args.log_level) + LOGGER.info(args) with logged_execution("Checking requirements"): check_requirements(args) From 1715d6d8f8a12eaed2a643ccefac0841d78aa3c0 Mon Sep 17 00:00:00 2001 From: Guilherme Caponetto <638737+caponetto@users.noreply.github.com> Date: Wed, 14 Aug 2024 14:26:31 -0300 Subject: [PATCH 12/12] Code review request --- scripts/new_python_based_image.py | 42 +++++++++++++------------------ 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/scripts/new_python_based_image.py b/scripts/new_python_based_image.py index 04557840a..6cf848171 100644 --- a/scripts/new_python_based_image.py +++ b/scripts/new_python_based_image.py @@ -13,7 +13,7 @@ LOGGER = logging.getLogger(__name__) -def configure_logger(log_level: str): +def configure_logger(log_level: str) -> None: """ Configures the logging settings based on the provided log level. @@ -40,14 +40,6 @@ class Args: match: str log_level: str - def __str__(self): - return (f"Arguments:\n" - f"Context Directory: {self.context_dir}\n" - f"Source Version: {self.source}\n" - f"Target Version: {self.target}\n" - f"Match: '{self.match}'\n" - f"Log Level: {self.log_level}") - def extract_input_args() -> Args: """ @@ -75,7 +67,7 @@ def extract_input_args() -> Args: help="The string to match with the paths to base the new image from.") parser.add_argument( "--log-level", default="INFO", - help="Set the logging level. Default: %(default)s).") + help="Set the logging level. Default: %(default)s.") args = parser.parse_args() @@ -88,7 +80,7 @@ def extract_input_args() -> Args: return Args(args.context_dir, args.source, args.target, args.match, args.log_level) -def extract_python_version(version: str) -> list: +def extract_python_version(version: str) -> list[str]: """ Extracts the major and minor version components from a Python version string. @@ -101,7 +93,7 @@ def extract_python_version(version: str) -> list: return version.split(".")[:2] -def check_python_version(version: str): +def check_python_version(version: str) -> None: """ Validates the format of a Python version string. @@ -114,7 +106,7 @@ def check_python_version(version: str): sys.exit(1) -def check_target_python_version_installed(version: str): +def check_target_python_version_installed(version: str) -> None: """ Checks if the specified Python version is installed on the system. @@ -127,7 +119,7 @@ def check_target_python_version_installed(version: str): sys.exit(1) -def check_input_versions_not_equal(source_version: str, target_version: str): +def check_input_versions_not_equal(source_version: str, target_version: str) -> None: """ Ensures that the source and target Python versions are different. @@ -140,7 +132,7 @@ def check_input_versions_not_equal(source_version: str, target_version: str): sys.exit(1) -def check_os_linux(): +def check_os_linux() -> None: """ Checks if the script is being run on a Linux operating system. """ @@ -151,7 +143,7 @@ def check_os_linux(): sys.exit(1) -def check_pipenv_installed(): +def check_pipenv_installed() -> None: """ Checks if `pipenv` is installed on the system. """ @@ -160,7 +152,7 @@ def check_pipenv_installed(): sys.exit(1) -def check_requirements(args: Args): +def check_requirements(args: Args) -> None: """ Performs various checks to ensure that all requirements are met. @@ -175,7 +167,7 @@ def check_requirements(args: Args): check_pipenv_installed() -def find_matching_paths(context_dir: str, source_version: str, match: str) -> list: +def find_matching_paths(context_dir: str, source_version: str, match: str) -> list[str]: """ Finds directories in the context directory that match the specified source version and match criteria. @@ -215,7 +207,7 @@ def find_matching_paths(context_dir: str, source_version: str, match: str) -> li return [p for p in matching_paths if source_version in p] -def replace_python_version_on_paths(paths_list: list, source_version: str, target_version: str) -> dict: +def replace_python_version_on_paths(paths_list: list, source_version: str, target_version: str) -> dict[str, str]: """ Replaces occurrences of the source Python version with the target version in a list of paths. @@ -230,7 +222,7 @@ def replace_python_version_on_paths(paths_list: list, source_version: str, targe return {path: path.replace(source_version, target_version) for path in paths_list} -def copy_paths(paths_dict: dict) -> tuple: +def copy_paths(paths_dict: dict) -> tuple[list[str], list[str]]: """ Copies directories from source paths to destination paths. @@ -261,7 +253,7 @@ def copy_paths(paths_dict: dict) -> tuple: return success_paths, failed_paths -def replace_python_version_in_file(file_path: str, source_version: str, target_version: str): +def replace_python_version_in_file(file_path: str, source_version: str, target_version: str) -> None: """ Replaces occurrences of the source Python version with the target version in a file. @@ -381,7 +373,7 @@ def replace_version_in_directory(directory_path: str, source_version: str, targe """ LOGGER.debug(f"Replacing Python versions in '{directory_path}'") - def rename_file_and_replace_python_version(path, filename, source_version, target_version, is_file=True): + def rename_file_and_replace_python_version(path, filename, source_version, target_version, is_file=True) -> str: old_path = os.path.join(path, filename) new_filename = replace_python_version_in_content(filename, source_version, @@ -416,7 +408,7 @@ def rename_file_and_replace_python_version(path, filename, source_version, targe is_file=False) -def process_paths(copied_paths: list, source_version: str, target_version: str) -> tuple: +def process_paths(copied_paths: list, source_version: str, target_version: str) -> tuple[list[str], list[str]]: """ Processes the list of copied paths by replacing Python versions and running `pipenv lock` on Pipfiles. @@ -442,7 +434,7 @@ def process_paths(copied_paths: list, source_version: str, target_version: str) return success_processed, failed_processed -def process_pipfiles(path: str, target_version: str) -> tuple: +def process_pipfiles(path: str, target_version: str) -> tuple[list[str], list[str]]: """ Processes Pipfiles in a given path by running `pipenv lock` on them. @@ -496,7 +488,7 @@ def run_pipenv_lock(pipfile_path: str, target_version: str) -> bool: return False -def manual_checks() -> list: +def manual_checks() -> list[str]: """ Provides a list of manual checks to perform after the script execution.