feat: add interactive systemd service generator (Bounty #448) #461
Conversation
📝 WalkthroughWalkthroughIntroduces a new interactive systemd service generator feature by adding a SystemdHelper class that prompts for service configuration, generates systemd unit file content, and optionally saves it to disk. Integrates this via a new "systemd" CLI command in CortexCLI. Changes
Sequence DiagramsequenceDiagram
actor User
participant CortexCLI
participant SystemdHelper
participant FileSystem
User->>CortexCLI: Invoke "systemd" command
CortexCLI->>SystemdHelper: Instantiate SystemdHelper
SystemdHelper->>User: Display wizard header
User->>SystemdHelper: Provide service name
User->>SystemdHelper: Provide description
User->>SystemdHelper: Provide ExecStart command
User->>SystemdHelper: Provide working directory
User->>SystemdHelper: Provide run user
User->>SystemdHelper: Provide Restart policy
SystemdHelper->>SystemdHelper: generate_service_content()
SystemdHelper->>User: Display generated service file preview
User->>SystemdHelper: Confirm save to disk
SystemdHelper->>FileSystem: Write .service file
FileSystem-->>SystemdHelper: Success/Error
SystemdHelper->>User: Display status & usage instructions
SystemdHelper-->>CortexCLI: Return completion status
CortexCLI-->>User: Exit with code 0 or 1
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
CLA Verification FailedThe following contributors have not signed the Contributor License Agreement:
How to Sign
This check runs automatically. Maintainers can update |
|
There was a problem hiding this comment.
Pull request overview
This PR implements an interactive CLI wizard for generating systemd service files, addressing bounty issue #448. The feature allows users to create valid systemd unit files through a guided "Plain English" interface accessible via cortex systemd or the alias cortex service-gen.
Key Changes:
- Added
SystemdHelperclass with interactive prompts for service configuration (name, description, command, working directory, user, restart policy) - Integrated the new command into the CLI with proper argparse setup and routing
- Added the command to the help documentation table
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| cortex/systemd_helper.py | New interactive wizard class that prompts users for service configuration and generates systemd unit file content with save functionality |
| cortex/cli.py | Integrates the systemd command into the CLI with command routing, argparse configuration, and help text entry |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def save_file(self, filename: str, content: str): | ||
| """Saves content to file.""" | ||
| try: | ||
| with open(filename, "w") as f: | ||
| f.write(content) | ||
| except Exception as e: | ||
| cx_print(f"Error saving file: {e}", "error") |
There was a problem hiding this comment.
The file saving operation lacks proper error handling and user feedback. When an exception occurs during file writing, the error is printed but the wizard continues to display "Next Steps" instructions as if the file was saved successfully. Consider returning early or raising the exception from save_file so the caller can handle the failure case appropriately and avoid showing misleading success instructions.
| import os | ||
| import getpass | ||
| from rich.prompt import Prompt, Confirm | ||
| from cortex.branding import console, cx_header, cx_print | ||
|
|
||
| class SystemdHelper: | ||
| """ | ||
| Interactive helper to generate systemd service files. | ||
| """ | ||
|
|
||
| def __init__(self): | ||
| self.service_name = "" | ||
| self.description = "" | ||
| self.exec_start = "" | ||
| self.working_dir = "" | ||
| self.user = "" | ||
| self.restart_policy = "always" | ||
|
|
||
| def run(self): | ||
| """Interactive wizard.""" | ||
| cx_header("Systemd Service Generator") | ||
|
|
||
| console.print("[dim]This wizard will help you create a systemd service file.[/dim]\n") | ||
|
|
||
| # 1. Service Name | ||
| self.service_name = Prompt.ask( | ||
| "[bold cyan]Service Name[/bold cyan] (e.g. myserver)", | ||
| default="myservice" | ||
| ) | ||
| if not self.service_name.endswith(".service"): | ||
| filename = f"{self.service_name}.service" | ||
| else: | ||
| filename = self.service_name | ||
| self.service_name = filename[:-8] | ||
|
|
||
| # 2. Description | ||
| self.description = Prompt.ask( | ||
| "[bold cyan]Description[/bold cyan]", | ||
| default=f"Service for {self.service_name}" | ||
| ) | ||
|
|
||
| # 3. ExecStart | ||
| self.exec_start = Prompt.ask( | ||
| "[bold cyan]Command to run (ExecStart)[/bold cyan]", | ||
| default="/usr/bin/python3 /path/to/app.py" | ||
| ) | ||
|
|
||
| # 4. Working Directory | ||
| current_dir = os.getcwd() | ||
| self.working_dir = Prompt.ask( | ||
| "[bold cyan]Working Directory[/bold cyan]", | ||
| default=current_dir | ||
| ) | ||
|
|
||
| # 5. User | ||
| current_user = getpass.getuser() | ||
| self.user = Prompt.ask( | ||
| "[bold cyan]Run as User[/bold cyan]", | ||
| default=current_user | ||
| ) | ||
|
|
||
| # 6. Restart Policy | ||
| self.restart_policy = Prompt.ask( | ||
| "[bold cyan]Restart Policy[/bold cyan]", | ||
| choices=["always", "on-failure", "no"], | ||
| default="always" | ||
| ) | ||
|
|
||
| # Generate Content | ||
| content = self.generate_service_content() | ||
|
|
||
| cx_header("Generated Content") | ||
| console.print(content, style="dim") | ||
| console.print() | ||
|
|
||
| # Save? | ||
| if Confirm.ask(f"Save to [bold green]{filename}[/bold green] in current directory?"): | ||
| self.save_file(filename, content) | ||
|
|
||
| cx_print(f"File saved: {filename}", "success") | ||
| console.print(f"\n[bold]Next Steps:[/bold]") | ||
| console.print(f"1. Move to systemd: [cyan]sudo mv {filename} /etc/systemd/system/[/cyan]") | ||
| console.print(f"2. Reload daemon: [cyan]sudo systemctl daemon-reload[/cyan]") | ||
| console.print(f"3. Enable service: [cyan]sudo systemctl enable {self.service_name}[/cyan]") | ||
| console.print(f"4. Start service: [cyan]sudo systemctl start {self.service_name}[/cyan]") | ||
|
|
||
| def generate_service_content(self) -> str: | ||
| """Generates the .service file content.""" | ||
| return f"""[Unit] | ||
| Description={self.description} | ||
| After=network.target | ||
| [Service] | ||
| Type=simple | ||
| User={self.user} | ||
| WorkingDirectory={self.working_dir} | ||
| ExecStart={self.exec_start} | ||
| Restart={self.restart_policy} | ||
| [Install] | ||
| WantedBy=multi-user.target | ||
| """ | ||
|
|
||
| def save_file(self, filename: str, content: str): | ||
| """Saves content to file.""" | ||
| try: | ||
| with open(filename, "w") as f: | ||
| f.write(content) | ||
| except Exception as e: | ||
| cx_print(f"Error saving file: {e}", "error") |
There was a problem hiding this comment.
The new systemd helper functionality lacks test coverage. Similar CLI commands in this repository have corresponding test files (e.g., test_cli.py, test_doctor.py, test_env_manager.py). Consider adding tests for the SystemdHelper class covering the interactive wizard flow, service file generation, and file saving operations.
| filename = f"{self.service_name}.service" | ||
| else: | ||
| filename = self.service_name | ||
| self.service_name = filename[:-8] |
There was a problem hiding this comment.
The service name extraction logic has a potential bug. When a user enters a service name ending with ".service", the code strips 8 characters (line 34), but it should strip 8 characters to get the name without the extension. However, if the user enters just ".service" or a very short name like "a.service", stripping 8 characters would leave an empty or negative-indexed string. Consider using filename.removesuffix(".service") (Python 3.9+) or filename[:-8] if len(filename) > 8 else "" with validation to ensure the service name is not empty.
| self.service_name = filename[:-8] | |
| stripped_name = filename.removesuffix(".service") | |
| # Avoid ending up with an empty service name (e.g. input ".service") | |
| if stripped_name: | |
| self.service_name = stripped_name | |
| else: | |
| self.service_name = filename |
|
|
||
| helper = SystemdHelper() | ||
| helper.run() | ||
| return 0 | ||
| except Exception as e: | ||
| self._print_error(f"Systemd Helper failed: {e}") |
There was a problem hiding this comment.
The error message lacks specificity. When catching a generic Exception, the error message only shows "Systemd Helper failed: {e}" without providing guidance on what went wrong or how to fix it. Consider catching specific exceptions (ImportError, KeyboardInterrupt, etc.) separately and providing more helpful error messages for common failure scenarios.
| helper = SystemdHelper() | |
| helper.run() | |
| return 0 | |
| except Exception as e: | |
| self._print_error(f"Systemd Helper failed: {e}") | |
| except ImportError as e: | |
| self._print_error( | |
| "Systemd Helper is not available. Make sure Cortex is installed with " | |
| "systemd support and that optional dependencies are installed.\n" | |
| f"Details: {e}" | |
| ) | |
| return 1 | |
| try: | |
| helper = SystemdHelper() | |
| helper.run() | |
| return 0 | |
| except KeyboardInterrupt: | |
| self._print_error("Systemd Helper cancelled by user.") | |
| return 1 | |
| except PermissionError as e: | |
| self._print_error( | |
| f"Permission error while running Systemd Helper: {e}\n" | |
| "Try running this command with elevated privileges or adjust your permissions." | |
| ) | |
| return 1 | |
| except Exception as e: | |
| logging.exception("Unexpected error while running Systemd Helper") | |
| self._print_error(f"Systemd Helper encountered an unexpected error: {e}") |
| console.print(f"1. Move to systemd: [cyan]sudo mv {filename} /etc/systemd/system/[/cyan]") | ||
| console.print(f"2. Reload daemon: [cyan]sudo systemctl daemon-reload[/cyan]") | ||
| console.print(f"3. Enable service: [cyan]sudo systemctl enable {self.service_name}[/cyan]") | ||
| console.print(f"4. Start service: [cyan]sudo systemctl start {self.service_name}[/cyan]") |
There was a problem hiding this comment.
When the user declines to save the file (answers "no" to the confirmation prompt), the wizard exits silently without any feedback. Consider adding an else clause to provide feedback to the user, such as informing them that the generated content was not saved or providing instructions on how to regenerate it if needed.
| console.print(f"4. Start service: [cyan]sudo systemctl start {self.service_name}[/cyan]") | |
| console.print(f"4. Start service: [cyan]sudo systemctl start {self.service_name}[/cyan]") | |
| else: | |
| cx_print("Generated service file was not saved. Rerun this wizard if you need to regenerate it.", "info") |
|
|
||
| # 1. Service Name | ||
| self.service_name = Prompt.ask( | ||
| "[bold cyan]Service Name[/bold cyan] (e.g. myserver)", |
There was a problem hiding this comment.
The example text shows "myserver" but this is inconsistent with the default value "myservice". Consider changing the example to match the default or vice versa for consistency.
| "[bold cyan]Service Name[/bold cyan] (e.g. myserver)", | |
| "[bold cyan]Service Name[/bold cyan] (e.g. myservice)", |
| self.restart_policy = Prompt.ask( | ||
| "[bold cyan]Restart Policy[/bold cyan]", | ||
| choices=["always", "on-failure", "no"], | ||
| default="always" | ||
| ) |
There was a problem hiding this comment.
The restart policy choice "no" should be "no" in the generated systemd file, but systemd actually expects "Restart=no" to be omitted or use other values. The valid values for the Restart directive are: "no", "on-success", "on-failure", "on-abnormal", "on-watchdog", "on-abort", or "always". While "no" is technically valid, consider adding "on-success" or "on-abnormal" as additional commonly used options to provide users with more flexibility.
| self.restart_policy = "always" | ||
|
|
||
| def run(self): | ||
| """Interactive wizard.""" |
There was a problem hiding this comment.
The method docstring is too brief and doesn't describe the parameters or behavior. Consider expanding it to document what the method does, that it's an interactive CLI wizard, and what it returns (always returns None implicitly). Also document any exceptions that might be raised during file operations.
| """Interactive wizard.""" | |
| """ | |
| Run the interactive CLI wizard to generate a systemd service file. | |
| This method interactively prompts the user for the service name, description, | |
| command to run (ExecStart), working directory, user, and restart policy. | |
| It then generates the corresponding systemd unit file content, displays it | |
| for review, and optionally writes the file to the current working directory. | |
| On successful save, it prints suggested next steps for installing and | |
| managing the service with systemd. | |
| The method does not accept any arguments other than ``self`` and always | |
| returns ``None``. | |
| File-writing errors that occur while saving the generated service file are | |
| caught and reported to the user by :meth:`save_file` and are not re-raised | |
| by this method. | |
| Raises: | |
| OSError: If the current working directory cannot be determined when | |
| calling :func:`os.getcwd`. | |
| """ |
| # 1. Service Name | ||
| self.service_name = Prompt.ask( | ||
| "[bold cyan]Service Name[/bold cyan] (e.g. myserver)", | ||
| default="myservice" | ||
| ) | ||
| if not self.service_name.endswith(".service"): | ||
| filename = f"{self.service_name}.service" | ||
| else: | ||
| filename = self.service_name | ||
| self.service_name = filename[:-8] | ||
|
|
||
| # 2. Description | ||
| self.description = Prompt.ask( | ||
| "[bold cyan]Description[/bold cyan]", | ||
| default=f"Service for {self.service_name}" | ||
| ) | ||
|
|
||
| # 3. ExecStart | ||
| self.exec_start = Prompt.ask( | ||
| "[bold cyan]Command to run (ExecStart)[/bold cyan]", | ||
| default="/usr/bin/python3 /path/to/app.py" | ||
| ) | ||
|
|
||
| # 4. Working Directory | ||
| current_dir = os.getcwd() | ||
| self.working_dir = Prompt.ask( | ||
| "[bold cyan]Working Directory[/bold cyan]", | ||
| default=current_dir | ||
| ) | ||
|
|
||
| # 5. User | ||
| current_user = getpass.getuser() | ||
| self.user = Prompt.ask( | ||
| "[bold cyan]Run as User[/bold cyan]", | ||
| default=current_user | ||
| ) | ||
|
|
||
| # 6. Restart Policy | ||
| self.restart_policy = Prompt.ask( | ||
| "[bold cyan]Restart Policy[/bold cyan]", | ||
| choices=["always", "on-failure", "no"], | ||
| default="always" | ||
| ) |
There was a problem hiding this comment.
The user inputs (service_name, exec_start, working_dir, user) are not validated before being written to the service file. While systemd will validate the file when it's loaded, consider adding basic validation to catch common errors early. For example: validate that the service_name doesn't contain invalid characters (spaces, special chars), that exec_start contains an executable path, that working_dir is an absolute path, and that the user exists on the system.
| def systemd(self) -> int: | ||
| """Interactive systemd service generator.""" | ||
| try: | ||
| from cortex.systemd_helper import SystemdHelper | ||
|
|
||
| helper = SystemdHelper() | ||
| helper.run() | ||
| return 0 | ||
| except Exception as e: | ||
| self._print_error(f"Systemd Helper failed: {e}") | ||
| return 1 |
There was a problem hiding this comment.
The new systemd command lacks integration testing in the CLI test suite. Other CLI commands have test coverage in test_cli.py or test_cli_extended.py. Consider adding tests that verify the systemd command is properly routed and that the SystemdHelper is instantiated and called correctly.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
cortex/systemd_helper.py (3)
43-46: Validate user input for ExecStart command.The ExecStart field accepts arbitrary command strings without validation. Invalid or malicious commands could be written to the service file. Consider adding basic validation to ensure the command path exists or warning the user about invalid paths.
🔎 Suggested validation enhancement
# 3. ExecStart self.exec_start = Prompt.ask( "[bold cyan]Command to run (ExecStart)[/bold cyan]", default="/usr/bin/python3 /path/to/app.py" ) + + # Validate command path exists (warn only, don't block) + cmd_parts = self.exec_start.split() + if cmd_parts: + cmd_path = cmd_parts[0] + if not os.path.isabs(cmd_path): + cx_print(f"Warning: '{cmd_path}' is not an absolute path. Systemd requires absolute paths for ExecStart.", "warning") + elif not os.path.exists(cmd_path): + cx_print(f"Warning: Command '{cmd_path}' does not exist on this system.", "warning")
48-53: Validate WorkingDirectory exists.The working directory input uses
os.getcwd()as default, which is good, but doesn't validate if a user-provided directory actually exists. Invalid directories will cause the systemd service to fail to start.🔎 Suggested validation
# 4. Working Directory current_dir = os.getcwd() self.working_dir = Prompt.ask( "[bold cyan]Working Directory[/bold cyan]", default=current_dir ) + + # Validate directory exists + if not os.path.isdir(self.working_dir): + cx_print(f"Warning: Directory '{self.working_dir}' does not exist.", "warning")
87-102: Consider sanitizing user inputs in service file content.User-provided values are directly interpolated into the systemd unit file without sanitization. While systemd is generally robust, special characters in description, paths, or commands could potentially cause parsing issues or unexpected behavior.
Consider adding basic sanitization, especially for the description field which could contain special characters. For production use, validate that:
- Paths don't contain newlines or control characters
- Description doesn't break the INI format
- Command strings are properly formatted
Example:
def _sanitize_description(self, desc: str) -> str: """Remove characters that could break INI format.""" return desc.replace('\n', ' ').replace('\r', ' ')cortex/cli.py (1)
1524-1534: Improve exception handling and docstring forsystemdmethod.Two improvements needed:
- The docstring should be more descriptive about what the command does
- Exception handling is too broad and doesn't handle
KeyboardInterruptseparately, which would be useful for allowing users to cancel the interactive wizard gracefully🔎 Proposed improvements
def systemd(self) -> int: - """Interactive systemd service generator.""" + """ + Interactive systemd service generator. + + Launches a wizard that prompts for service configuration and generates + a systemd unit file with installation instructions. + + Returns: + 0 on success, 1 on failure + """ try: from cortex.systemd_helper import SystemdHelper helper = SystemdHelper() helper.run() return 0 + except KeyboardInterrupt: + console.print("\n[yellow]Wizard cancelled[/yellow]") + return 130 # Standard exit code for SIGINT - except Exception as e: + except (ImportError, OSError) as e: self._print_error(f"Systemd Helper failed: {e}") return 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cortex/cli.pycortex/systemd_helper.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide
Type hints required in Python code
Docstrings required for all public APIs
Files:
cortex/cli.pycortex/systemd_helper.py
🧬 Code graph analysis (2)
cortex/cli.py (1)
cortex/systemd_helper.py (2)
SystemdHelper(6-110)run(19-85)
cortex/systemd_helper.py (1)
cortex/branding.py (2)
cx_header(82-88)cx_print(49-69)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (3)
cortex/cli.py (3)
1568-1568: LGTM!The help table entry is clear and follows the existing pattern for other commands.
1873-1877: LGTM!The argument parser setup is clean and follows existing patterns. The alias "service-gen" provides a convenient alternative command name.
1924-1925: LGTM!The command dispatch correctly handles both the main command name and its alias, routing them to the appropriate handler method.
| def __init__(self): | ||
| self.service_name = "" | ||
| self.description = "" | ||
| self.exec_start = "" | ||
| self.working_dir = "" | ||
| self.user = "" | ||
| self.restart_policy = "always" |
There was a problem hiding this comment.
Add type hints and docstring to __init__ method.
The __init__ method lacks a docstring and return type hint, which violates the coding guidelines requiring type hints and docstrings for all public APIs.
🔎 Proposed fix
- def __init__(self):
+ def __init__(self) -> None:
+ """Initialize the SystemdHelper with default values for service configuration."""
self.service_name = ""
self.description = ""
self.exec_start = ""
self.working_dir = ""
self.user = ""
self.restart_policy = "always"Based on coding guidelines, type hints and docstrings are required for all public APIs.
🤖 Prompt for AI Agents
In cortex/systemd_helper.py around lines 11 to 17, the __init__ method is
missing a docstring and type hints; update the method signature to include an
explicit return type of None, add inline type annotations for the instance
attributes (e.g., self.service_name: str = "", self.description: str = "",
self.exec_start: str = "", self.working_dir: str = "", self.user: str = "",
self.restart_policy: str = "always"), and add a concise docstring that describes
the initializer and documents the attributes being set; keep the implementation
behavior unchanged.
| def run(self): | ||
| """Interactive wizard.""" |
There was a problem hiding this comment.
Add return type hint and improve docstring.
The run method lacks a return type hint and has a minimal docstring that doesn't describe the interactive wizard flow or its purpose in detail.
🔎 Proposed fix
- def run(self):
- """Interactive wizard."""
+ def run(self) -> None:
+ """
+ Run the interactive wizard to collect systemd service configuration.
+
+ Prompts the user for service name, description, command, working directory,
+ user, and restart policy. Generates the service file content and optionally
+ saves it to disk with follow-up instructions.
+ """Based on coding guidelines, type hints are required in Python code and docstrings are required for all public APIs.
🤖 Prompt for AI Agents
In cortex/systemd_helper.py around lines 19 to 20, the public method run() is
missing a return type hint and has an underspecified docstring; update the
signature to include an explicit return type (e.g., -> None or an appropriate
type if it returns a value) and replace the one-line docstring with a short
multi-sentence docstring that states the method is an interactive wizard,
describes its purpose, outlines the high-level flow (what prompts it shows and
what it returns or persists), and documents side effects and return value;
ensure the docstring follows project docstring conventions (reST/Google style)
and include type information for the return and any important parameters.
| self.service_name = Prompt.ask( | ||
| "[bold cyan]Service Name[/bold cyan] (e.g. myserver)", | ||
| default="myservice" | ||
| ) | ||
| if not self.service_name.endswith(".service"): | ||
| filename = f"{self.service_name}.service" | ||
| else: | ||
| filename = self.service_name | ||
| self.service_name = filename[:-8] |
There was a problem hiding this comment.
Handle edge case when user enters exactly ".service" as service name.
If a user enters exactly ".service" as the service name, line 34 will set self.service_name to an empty string (".service"[:-8] = ""), which would result in an invalid service name and could cause issues later.
🔎 Proposed fix to validate service name
self.service_name = Prompt.ask(
"[bold cyan]Service Name[/bold cyan] (e.g. myserver)",
default="myservice"
)
+
+ # Strip .service suffix if present
if not self.service_name.endswith(".service"):
filename = f"{self.service_name}.service"
else:
filename = self.service_name
self.service_name = filename[:-8]
+
+ # Validate service name is not empty
+ if not self.service_name or not self.service_name.strip():
+ cx_print("Invalid service name. Using default 'myservice'", "warning")
+ self.service_name = "myservice"
+ filename = "myservice.service"🤖 Prompt for AI Agents
In cortex/systemd_helper.py around lines 26 to 34, the current logic turns an
input of ".service" into an empty service name; after reading the Prompt input,
strip whitespace, then if the input equals ".service" or the computed base name
(input with a trailing ".service" removed) is empty, treat it as invalid —
either re-prompt the user or fall back to a sane default (e.g., "myservice");
otherwise, if input endswith ".service" strip the suffix and use the remaining
non-empty name, and if it does not endwith ".service" use the stripped input,
ensuring you never assign an empty string to self.service_name.
| def save_file(self, filename: str, content: str): | ||
| """Saves content to file.""" | ||
| try: | ||
| with open(filename, "w") as f: | ||
| f.write(content) | ||
| except Exception as e: | ||
| cx_print(f"Error saving file: {e}", "error") |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Improve save_file method with type hints, encoding, and return value.
The method has several issues:
- Missing type hints (violates coding guidelines)
- No explicit encoding specified for file writing (should be UTF-8)
- Overly broad exception handling catches all exceptions
- No return value to indicate success/failure to caller
🔎 Proposed improvements
- def save_file(self, filename: str, content: str):
- """Saves content to file."""
+ def save_file(self, filename: str, content: str) -> bool:
+ """
+ Save content to file.
+
+ Args:
+ filename: Path where the file should be saved
+ content: Content to write to the file
+
+ Returns:
+ True if file was saved successfully, False otherwise
+ """
try:
- with open(filename, "w") as f:
+ with open(filename, "w", encoding="utf-8") as f:
f.write(content)
- except Exception as e:
+ return True
+ except (OSError, IOError) as e:
cx_print(f"Error saving file: {e}", "error")
+ return FalseBased on coding guidelines, type hints are required in Python code.
🤖 Prompt for AI Agents
In cortex/systemd_helper.py around lines 104 to 110, the save_file method lacks
type hints, does not specify file encoding, catches all exceptions too broadly,
and returns nothing; change the signature to include type hints for parameters
and a boolean return (e.g., def save_file(self, filename: str, content: str) ->
bool), open the file with encoding="utf-8" when writing, catch only IO-related
exceptions (OSError or IOError), log the error message, and return False on
failure and True on success so callers can react to the result.
Anshgrover23
left a comment
Anshgrover23
left a comment
There was a problem hiding this comment.
@murataslan1 This PR doesn’t follow the contributing.md guidelines yet.
Tests, documentation, a demo video, and the AI usage section in the PR description are missing.
Marking this as draft until those are added. Please update and ping once done.
3 participants
Closes #448. Implements a 'Plain English' CLI wizard to generate valid systemd unit files. Usage: cortex systemd
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.