From fcfb349d2994caaf9a6af1f9871ea64bee8090d6 Mon Sep 17 00:00:00 2001 From: Walter Boring Date: Fri, 27 Feb 2026 23:35:38 -0500 Subject: [PATCH] Fix CLI command inconsistencies - Refactor duplicate plugin discovery code into aprsd/utils/package.py - Fix inconsistent --profile option in listen.py (now uses common_options) - Add common_options decorator to completion command for consistency - Improve healthcheck error message for missing APRSClientStats - Consolidate signal handler in listen.py to use shared one from main.py --- aprsd/cmds/completion.py | 5 +++- aprsd/cmds/export_plugins.py | 44 +----------------------------- aprsd/cmds/healthcheck.py | 2 +- aprsd/cmds/list_plugins.py | 39 +++++++++++---------------- aprsd/cmds/listen.py | 52 +++--------------------------------- aprsd/utils/package.py | 37 +++++++++++++++++++++++++ 6 files changed, 62 insertions(+), 117 deletions(-) diff --git a/aprsd/cmds/completion.py b/aprsd/cmds/completion.py index c118817..8b189c3 100644 --- a/aprsd/cmds/completion.py +++ b/aprsd/cmds/completion.py @@ -1,16 +1,19 @@ import click import click.shell_completion +from aprsd import cli_helper from aprsd.main import cli CONTEXT_SETTINGS = dict(help_option_names=['-h', '--help']) @cli.command() +@cli_helper.add_options(cli_helper.common_options) @click.argument( 'shell', type=click.Choice(list(click.shell_completion._available_shells)) ) -def completion(shell): +@cli_helper.process_standard_options_no_config +def completion(ctx, shell): """Show the shell completion code""" from click.utils import _detect_program_name diff --git a/aprsd/cmds/export_plugins.py b/aprsd/cmds/export_plugins.py index 0af07be..f06ae02 100644 --- a/aprsd/cmds/export_plugins.py +++ b/aprsd/cmds/export_plugins.py @@ -7,53 +7,11 @@ import click from aprsd import cli_helper from aprsd import plugin as aprsd_plugin from aprsd.main import cli -from aprsd.plugins import fortune, notify, ping, time, version, weather from aprsd.utils import package as aprsd_package LOG = logging.getLogger('APRSD') -def get_built_in_plugins(): - """Discover all built-in APRSD plugins.""" - modules = [fortune, notify, ping, time, version, weather] - plugins = [] - - for module in modules: - entries = inspect.getmembers(module, inspect.isclass) - for entry in entries: - cls = entry[1] - if issubclass(cls, aprsd_plugin.APRSDPluginBase): - plugin_info = { - 'package': 'aprsd', - 'class_name': cls.__qualname__, - 'path': f'{cls.__module__}.{cls.__qualname__}', - 'version': cls.version, - 'base_class_type': aprsd_package.plugin_type(cls), - } - - # If it's a regex command plugin, include the command_regex - if issubclass(cls, aprsd_plugin.APRSDRegexCommandPluginBase): - # Try to get command_regex from the class - # It's typically defined as a class attribute in plugin implementations - try: - # Check the MRO to find where command_regex is actually defined - cmd_regex = None - for base_cls in inspect.getmro(cls): - if 'command_regex' in base_cls.__dict__: - attr = base_cls.__dict__['command_regex'] - # If it's not a property descriptor, use it - if not isinstance(attr, property): - cmd_regex = attr - break - plugin_info['command_regex'] = cmd_regex - except Exception: - plugin_info['command_regex'] = None - - plugins.append(plugin_info) - - return plugins - - def get_installed_plugin_classes(): """Discover all installed 3rd party plugin classes, grouped by package.""" installed_plugins = aprsd_package.get_installed_plugins() @@ -117,7 +75,7 @@ def export_plugins(ctx): } # Get built-in plugins - built_in = get_built_in_plugins() + built_in = aprsd_package.get_built_in_plugins() output['built_in_plugins'] = built_in # Get installed 3rd party plugins (grouped by package) diff --git a/aprsd/cmds/healthcheck.py b/aprsd/cmds/healthcheck.py index d97b9b5..5515117 100644 --- a/aprsd/cmds/healthcheck.py +++ b/aprsd/cmds/healthcheck.py @@ -85,7 +85,7 @@ def healthcheck(ctx, timeout): client_stats = stats.get('APRSClientStats') if not client_stats: - console.log('No APRSClientStats') + console.log('No APRSClientStats - Is the aprsd server running?') sys.exit(-1) else: aprsis_last_update = client_stats['connection_keepalive'] diff --git a/aprsd/cmds/list_plugins.py b/aprsd/cmds/list_plugins.py index b2f590c..5e006b3 100644 --- a/aprsd/cmds/list_plugins.py +++ b/aprsd/cmds/list_plugins.py @@ -1,4 +1,3 @@ -import inspect import logging import click @@ -7,39 +6,33 @@ from rich.table import Table from rich.text import Text from aprsd import cli_helper -from aprsd import plugin as aprsd_plugin from aprsd.main import cli -from aprsd.plugins import fortune, notify, ping, time, version, weather from aprsd.utils import package as aprsd_package LOG = logging.getLogger('APRSD') def show_built_in_plugins(console): - modules = [fortune, notify, ping, time, version, weather] + built_in = aprsd_package.get_built_in_plugins() plugins = [] - for module in modules: - entries = inspect.getmembers(module, inspect.isclass) - for entry in entries: - cls = entry[1] - if issubclass(cls, aprsd_plugin.APRSDPluginBase): - info = { - 'name': cls.__qualname__, - 'path': f'{cls.__module__}.{cls.__qualname__}', - 'version': cls.version, - 'docstring': cls.__doc__, - 'short_desc': cls.short_description, - } + for plugin in built_in: + info = { + 'name': plugin['class_name'], + 'path': plugin['path'], + 'version': plugin['version'], + 'short_desc': '', + } - if issubclass(cls, aprsd_plugin.APRSDRegexCommandPluginBase): - info['command_regex'] = cls.command_regex - info['type'] = 'RegexCommand' + if plugin.get('command_regex'): + info['command_regex'] = plugin['command_regex'] + info['type'] = 'RegexCommand' + elif plugin['base_class_type'] == 'WatchList': + info['type'] = 'WatchList' + else: + info['type'] = plugin['base_class_type'] - if issubclass(cls, aprsd_plugin.APRSDWatchListPluginBase): - info['type'] = 'WatchList' - - plugins.append(info) + plugins.append(info) plugins = sorted(plugins, key=lambda i: i['name']) diff --git a/aprsd/cmds/listen.py b/aprsd/cmds/listen.py index accf790..d27369b 100644 --- a/aprsd/cmds/listen.py +++ b/aprsd/cmds/listen.py @@ -1,10 +1,6 @@ -import cProfile -import datetime import logging -import pstats import signal import sys -import time import click from loguru import logger @@ -34,16 +30,9 @@ console = Console() def signal_handler(sig, frame): - threads.APRSDThreadList().stop_all() - if 'subprocess' not in str(frame): - LOG.info( - 'Ctrl+C, Sending all threads exit! Can take up to 10 seconds {}'.format( - datetime.datetime.now(), - ), - ) - time.sleep(5) - # Last save to disk - collector.Collector().collect() + from aprsd import main as aprsd_main + + aprsd_main.signal_handler(sig, frame) class APRSDListenProcessThread(rx.APRSDFilterThread): @@ -154,12 +143,6 @@ class APRSDListenProcessThread(rx.APRSDFilterThread): default='http://localhost:8081', help='URL of the aprsd-exporter API to send stats to.', ) -@click.option( - '--profile', - default=False, - is_flag=True, - help='Enable Python cProfile profiling to identify performance bottlenecks.', -) @click.pass_context @cli_helper.process_standard_options def listen( @@ -174,7 +157,6 @@ def listen( enable_packet_stats, export_stats, exporter_url, - profile, ): """Listen to packets on the APRS-IS Network based on FILTER. @@ -186,12 +168,6 @@ def listen( o/obj1/obj2... - Object Filter Pass all objects with the exact name of obj1, obj2, ... (* wild card allowed)\n """ - # Initialize profiler if enabled - profiler = None - if profile: - LOG.info('Starting Python cProfile profiling') - profiler = cProfile.Profile() - profiler.enable() signal.signal(signal.SIGINT, signal_handler) signal.signal(signal.SIGTERM, signal_handler) @@ -315,25 +291,3 @@ def listen( stats.join() if stats_export: stats_export.join() - - # Save profiling results if enabled - if profiler: - profiler.disable() - profile_file = 'aprsd_listen_profile.prof' - profiler.dump_stats(profile_file) - LOG.info(f'Profile saved to {profile_file}') - - # Print profiling summary - LOG.info('Profile Summary (top 50 functions by cumulative time):') - stats = pstats.Stats(profiler) - stats.sort_stats('cumulative') - - # Log the top functions - LOG.info('-' * 80) - for item in stats.get_stats().items()[:50]: - func_info, stats_tuple = item - cumulative = stats_tuple[3] - total_calls = stats_tuple[0] - LOG.info( - f'{func_info} - Calls: {total_calls}, Cumulative: {cumulative:.4f}s' - ) diff --git a/aprsd/utils/package.py b/aprsd/utils/package.py index fd87f7d..830f13b 100644 --- a/aprsd/utils/package.py +++ b/aprsd/utils/package.py @@ -11,6 +11,7 @@ import requests from thesmuggler import smuggle from aprsd import plugin as aprsd_plugin +from aprsd.plugins import fortune, notify, ping, time, version, weather # Handle importlib.metadata compatibility try: @@ -245,3 +246,39 @@ def log_installed_extensions_and_plugins(): for plugin in plugins: LOG.info(f'Plugin: {plugin} version: {plugins[plugin][0]["version"]}') + + +def get_built_in_plugins(): + """Discover all built-in APRSD plugins.""" + modules = [fortune, notify, ping, time, version, weather] + plugins = [] + + for module in modules: + entries = inspect.getmembers(module, inspect.isclass) + for entry in entries: + cls = entry[1] + if issubclass(cls, aprsd_plugin.APRSDPluginBase): + plugin_info = { + 'package': 'aprsd', + 'class_name': cls.__qualname__, + 'path': f'{cls.__module__}.{cls.__qualname__}', + 'version': cls.version, + 'base_class_type': plugin_type(cls), + } + + if issubclass(cls, aprsd_plugin.APRSDRegexCommandPluginBase): + try: + cmd_regex = None + for base_cls in inspect.getmro(cls): + if 'command_regex' in base_cls.__dict__: + attr = base_cls.__dict__['command_regex'] + if not isinstance(attr, property): + cmd_regex = attr + break + plugin_info['command_regex'] = cmd_regex + except Exception: + plugin_info['command_regex'] = None + + plugins.append(plugin_info) + + return plugins