Skip to content

Automatically add reload-* cmd that performs teardown-* + setup-* #348

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ezio-melotti wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Automatically add reload-* cmd that performs teardown-* + setup-* #348

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
41ab8ed
Automatically add reload-* cmd that performs teardown-* + setup-*
ezio-melotti Mar 25, 2026
cf04568
Fix command display name in docstring
ezio-melotti Mar 25, 2026
ef8b1fb
Remove two extra decorators
ezio-melotti Mar 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 24 additions & 3 deletions simoc-sam.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import shutil
import socket
import pathlib
import inspect
import argparse
import datetime
import tempfile
Expand Down Expand Up @@ -49,9 +50,31 @@

COMMANDS = {}

def add_reload_function(cmd):
"""Add a reload-cmd function that performs teardown-cmd + setup-cmd."""
teardown_func = COMMANDS.get(f'teardown_{cmd}')
setup_func = COMMANDS.get(f'setup_{cmd}')
def reload_func(*args, **kwargs):
if inspect.signature(teardown_func).parameters:
teardown_func(*args, **kwargs)
else:
teardown_func()
return setup_func(*args, **kwargs)
Comment on lines +59 to +62

Copilot AI Mar 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reload_func ignores the result of teardown_* and always proceeds to setup_*, returning only the setup result. This can mask teardown failures (and can also run setup against a partially-torn-down state). Consider capturing the teardown return value and short-circuiting on an explicit False result (while treating None as success), and combining teardown+setup outcomes in the returned status.

Suggested change
teardown_func(*args, **kwargs)
else:
teardown_func()
return setup_func(*args, **kwargs)
teardown_result = teardown_func(*args, **kwargs)
else:
teardown_result = teardown_func()
# If teardown explicitly reports failure, do not proceed to setup.
if teardown_result is False:
return False
setup_result = setup_func(*args, **kwargs)
# Treat None as success; otherwise, use the truthiness of the result.
teardown_ok = True if teardown_result is None else bool(teardown_result)
setup_ok = True if setup_result is None else bool(setup_result)
return teardown_ok and setup_ok

Copilot uses AI. Check for mistakes.
reload_func_name = f'reload_{cmd}'
hyphen_cmd = cmd.replace('_', '-')
reload_func.__name__ = reload_func_name
reload_func.__doc__ = f"Same as teardown-{hyphen_cmd} + setup-{hyphen_cmd}."
COMMANDS[reload_func_name] = reload_func

def cmd(func):
"""Decorator to add commands to the COMMANDS dict."""
COMMANDS[func.__name__] = func
func_name = func.__name__
COMMANDS[func_name] = func
if func_name.startswith('teardown_'):
suffix = func_name.removeprefix('teardown_')
if f'setup_{suffix}' in COMMANDS:
# if both setup-* and teardown-* exist, add reload-*
add_reload_function(suffix)
Comment thread
ezio-melotti marked this conversation as resolved.
Comment on lines +73 to +77

Copilot AI Mar 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reload-* command is only added when a teardown_* function is registered after its corresponding setup_* is already in COMMANDS. If a future command pair is declared in the opposite order (teardown_* before setup_*), no reload-* alias will be created. To make this truly “automatic for each pair”, consider adding the same pairing logic when registering setup_* (or doing a final pairing pass after all commands are registered).

Suggested change
if func_name.startswith('teardown_'):
suffix = func_name.removeprefix('teardown_')
if f'setup_{suffix}' in COMMANDS:
# if both setup-* and teardown-* exist, add reload-*
add_reload_function(suffix)
# If this is a setup_* or teardown_* command, and both sides of the pair
# are registered, automatically add a corresponding reload_* command.
suffix = None
if func_name.startswith('teardown_'):
suffix = func_name.removeprefix('teardown_')
elif func_name.startswith('setup_'):
suffix = func_name.removeprefix('setup_')
if suffix:
setup_name = f'setup_{suffix}'
teardown_name = f'teardown_{suffix}'
reload_name = f'reload_{suffix}'
if setup_name in COMMANDS and teardown_name in COMMANDS and reload_name not in COMMANDS:
# if both setup-* and teardown-* exist, add reload-* (once)
add_reload_function(suffix)

Copilot uses AI. Check for mistakes.
return func

def run(args, **kwargs):
Expand Down Expand Up @@ -386,7 +409,6 @@ def teardown_systemd_unit(name, unit_type='service', stop=True, disable=True):
pathlib.Path(SYSTEMD_DIR / unit_name).unlink(missing_ok=True)


@cmd
@needs_root

Copilot AI Mar 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that setup_or_teardown_sensors is no longer a CLI command, keeping @needs_root on it is redundant (it’s only called from @needs_root commands) and can be surprising if the helper is reused elsewhere (it may execvp sudo). Consider removing @needs_root from the helper and letting only the top-level commands enforce privilege escalation.

Suggested change
@needs_root

Copilot uses AI. Check for mistakes.
def setup_or_teardown_sensors(function, sensors=None):
"""Setup systemd services that run the sensors."""
Expand All @@ -409,7 +431,6 @@ def teardown_sensors(sensors=None):
"""Revert the changes made by the setup-sensors command."""
setup_or_teardown_sensors(teardown_systemd_unit, sensors)

@cmd
@needs_root

Copilot AI Mar 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that setup_or_teardown_display is no longer a CLI command, @needs_root on this helper is redundant (callers already enforce root) and can be surprising if reused (it may execvp sudo). Consider moving the root requirement to only the public setup-*/teardown-* commands and keeping helpers undecorated.

Suggested change
@needs_root

Copilot uses AI. Check for mistakes.
def setup_or_teardown_display(function, display=None):
"""Setup/teardown systemd service that runs the display."""
Expand Down
Loading