This is the second time I’ve asked you to review my code. This time I used codepal.ai and the famous linters for Python to make it easier for you to review my code. I wanted to be fair in the first review, since I didn’t try to improve it. However, I did learn something from the first review, such as the magic number and the importance of error handling. With your insight and ChatGPT, I’m confident I can learn more efficiently and improve my code. Any criticism or insight is welcome.
import argparse
import logging
import secrets
import subprocess
import sys
# Set up logging
logging.basicConfig(
level=logging.INFO, format="\n%(asctime)s - %(levelname)s - %(message)s"
)
logger = logging.getLogger(__name__)
logger.info("Logging initialized")
# Constants
YES = "yes"
EXIT = "exit"
CHANGE = "change"
MAX_NUMBER_LENGTH = 2
# Run the command
COMMAND = "/bin/echo"
ARGS = "Hello, World!"
# Check the output
EXPECTED_OUTPUT = "Hello, World!"
# Define constant tuples
PREDEFINED_STRINGS = (
"Why did Python developers always write unit tests first? "
"Because they don't want to get bit by a bug!",
"Why did the Python script get a job in testing? "
"Because it was an expert in handling exceptions!",
"Why did the Python script cross the road? "
"To get to the other side of the integration test!",
"Why did the Python script fail the integration test? "
"Because it couldn't handle the stress of working with other modules!",
"Why did the Python script get lost in the testing environment? "
"Because it forgot to import the os module!",
"Why did the Python script fail the performance test? "
"Because it was too slow, like a python in hibernation!",
)
FRUITS_LIST_FILE = "./fruits.list"
class CalledProcessError(Exception):
"""
Exception raised when a command fails.
"""
def __init__(self, message):
"""
Initializes the exception.
Args:
message (str): The error message.
"""
self.message = message
def run_command(command: str, *args: str) -> str:
"""
Executes a command with arguments securely.
Args:
command (str): The command to execute.
*args (str): The arguments to pass to the command.
Returns:
str: The output of the command.
Raises:
ValueError: If the command or any of the arguments are not strings.
CalledProcessError: If the command fails.
OSError: If an OS-level error occurs while running the command.
"""
logger.info("Running command %s with string %s", command, args)
try:
if not isinstance(command, str):
raise ValueError("Command must be a string")
if not all(isinstance(arg, str) for arg in args):
raise ValueError("All arguments must be strings")
# Sanitize input
command = command.strip()
args = tuple(arg.strip() for arg in args)
# Use subprocess securely
result = subprocess.run(
[command, *args],
check=True,
shell=False,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
stdin=subprocess.PIPE,
timeout=5,
text=True,
encoding="utf-8",
)
# Log output
logging.info(
"Command '%s %s' executed successfully with output: %s",
command,
args,
result.stdout,
)
except subprocess.CalledProcessError as error:
logging.error(
"Command '%s %s' failed with return code: %s\n%s",
command,
args,
error.returncode,
error.stderr,
)
raise CalledProcessError(
f"Command '{command} {args}' failed with return code: "
"{error.returncode}\n{error.stderr}"
) from error
except OSError as error:
logging.error(
"An OS-level error occurred while running the command '%s %s': %s",
command,
args,
error,
)
raise OSError(
"An OS-level error occurred while running "
f"the command '{command} {args}': {error}"
) from error
return result.stdout
def prompt_user_for_action():
"""Prompts the user for an action.
Returns:
str: The user's input.
"""
try:
while True:
answer = input("Options: Yes, Exit, Change\nProceed? ").lower()
if answer == YES:
break
if answer == EXIT:
logger.info("User exited the script")
sys.exit(0)
if answer == CHANGE:
change_user_input()
else:
logger.error("The answer should be (yes|exit|change) only.")
except ValueError:
logger.error("That was not a valid input. Please try again.")
except KeyboardInterrupt as err:
raise KeyboardInterrupt(
"User interrupted the script. Exiting..."
) from err
def change_user_input():
"""Prompts the user for a new number and updates the argument."""
while True:
try:
user_input = input(
f"Please enter a new number (max {MAX_NUMBER_LENGTH} digits): "
)
while len(user_input) > MAX_NUMBER_LENGTH:
logger.error(
"Number must be at most %s digits long.", MAX_NUMBER_LENGTH
)
user_input = input("Enter an integer: ")
# Convert the input to an integer
params.u = int(user_input)
logger.info("User input changed to %s", params.u)
break
except ValueError:
logger.error("That was not a valid integer. Please try again.")
except KeyboardInterrupt as error:
raise KeyboardInterrupt(
"User interrupted the script. Exiting..."
) from error
def get_random_fruit():
"""
Reads the fruits list file and returns a random fruit from it.
Logs an error if the file is not found.
"""
try:
with open(FRUITS_LIST_FILE, "r", encoding="utf-8") as fruits_list_file:
lines = fruits_list_file.readlines()
random_fruit = secrets.choice(lines)
fruit_string = random_fruit.strip()
return fruit_string
except FileNotFoundError:
logger.error("Fruits list file not found: %s", FRUITS_LIST_FILE)
sys.exit(2)
def check_arguments(parsed_args):
"""Check if the given arguments are valid.
Args:
parsed_args (argparse.Namespace): The parsed arguments
Raises:
SystemExit: If the given arguments are not valid
"""
try:
if parsed_args.r and parsed_args.t:
error_message = "Arguments -r and -t cannot be used together."
logger.error(error_message)
raise SystemExit(error_message)
if parsed_args.A and parsed_args.R:
error_message = "Arguments -R and -A cannot be used together."
logger.error(error_message)
raise SystemExit(error_message)
logger.info("Arguments checked successfully")
except Exception as error:
logger.error("Error checking arguments: %s", error)
raise SystemExit(f"Error checking arguments: {error}") from error
def parse_command_line_arguments():
"""Parse command-line arguments.
Returns:
argparse.Namespace: The parsed arguments
"""
# Parse command-line arguments.
parser = argparse.ArgumentParser(description="Process a few arguments.")
parser.add_argument(
"-u", type=int, help="Number to be entered", metavar=""
)
parser.add_argument("-r", action="store_true", help="...")
parser.add_argument("-R", action="store_true", help="...")
parser.add_argument("-s", action="store_true", help="...")
parser.add_argument("-A", action="store_true", help="...")
parser.add_argument("-l", action="store_true", help="...")
parser.add_argument("-t", action="store_true", help="...")
parser.add_argument("-d", action="store_true", help="...")
args = parser.parse_args()
return args
if __name__ == "__main__":
try:
params = parse_command_line_arguments()
check_arguments(params)
logger.info("Arguments parsed successfully")
except SystemExit as exc:
logger.error("Error parsing arguments: %s", exc)
sys.exit(2)
output = run_command(COMMAND, ARGS)
if EXPECTED_OUTPUT not in output:
logging.error(
"The command '%s %s' did not return the expected output",
COMMAND,
ARGS,
)
sys.exit(1)
else:
logging.info(
"Command executed successfully and returned expected output"
)
try:
prompt_user_for_action()
except KeyboardInterrupt:
logger.info("User interrupted the script. Exiting...")
sys.exit(0)
if params.r:
fruit = get_random_fruit()
run_command("/bin/echo", f"{fruit}")
if params.R:
run_command("/bin/echo", PREDEFINED_STRINGS[0])
if params.s:
run_command("/bin/echo", PREDEFINED_STRINGS[1])
if params.A:
run_command("/bin/echo", PREDEFINED_STRINGS[2])
if params.l:
run_command("/bin/echo", PREDEFINED_STRINGS[3])
if params.t:
run_command("/bin/echo", PREDEFINED_STRINGS[4])
if params.d:
run_command("/bin/echo", PREDEFINED_STRINGS[5])