From 8e18ff1d08f7e36d67300faf4c83f38200f82219 Mon Sep 17 00:00:00 2001 From: Daniel Languiller Date: Wed, 10 Dec 2025 16:09:18 +1100 Subject: [PATCH 01/18] wip: using threads to build command table --- .vscode/launch.json | 2 +- src/azure-cli-core/azure/cli/core/__init__.py | 56 ++++++++++++------- .../azure/cli/core/commands/__init__.py | 1 + 3 files changed, 39 insertions(+), 20 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index e7ed7a11353..88410318724 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -23,7 +23,7 @@ "program": "${workspaceRoot}/src/azure-cli/azure/cli/__main__.py", "cwd": "${workspaceRoot}", "args": [ - "--version" + "Version" ], "console": "externalTerminal", }, diff --git a/src/azure-cli-core/azure/cli/core/__init__.py b/src/azure-cli-core/azure/cli/core/__init__.py index 0783ed923ac..32d4f4675b8 100644 --- a/src/azure-cli-core/azure/cli/core/__init__.py +++ b/src/azure-cli-core/azure/cli/core/__init__.py @@ -9,6 +9,7 @@ import os import sys import timeit +from concurrent.futures import ThreadPoolExecutor from knack.cli import CLI from knack.commands import CLICommandsLoader @@ -260,31 +261,48 @@ def _update_command_table_from_modules(args, command_modules=None): logger.debug("Loading command modules:") logger.debug(self.header_mod) - for mod in [m for m in command_modules if m not in BLOCKED_MODS]: + print(f"*** Starting SUT***") + start_time_wrapper = timeit.default_timer() + + def load_module_threaded(mod): try: start_time = timeit.default_timer() module_command_table, module_group_table = _load_module_command_loader(self, args, mod) import_module_breaking_changes(mod) - for cmd in module_command_table.values(): - cmd.command_source = mod - self.command_table.update(module_command_table) - self.command_group_table.update(module_group_table) - elapsed_time = timeit.default_timer() - start_time - logger.debug(self.item_format_string, mod, elapsed_time, - len(module_group_table), len(module_command_table)) - count += 1 - cumulative_elapsed_time += elapsed_time - cumulative_group_count += len(module_group_table) - cumulative_command_count += len(module_command_table) + return (mod, module_command_table, module_group_table, elapsed_time, None) except Exception as ex: # pylint: disable=broad-except - # Changing this error message requires updating CI script that checks for failed - # module loading. - from azure.cli.core import telemetry - logger.error("Error loading command module '%s': %s", mod, ex) - telemetry.set_exception(exception=ex, fault_type='module-load-error-' + mod, - summary='Error loading module: {}'.format(mod)) - logger.debug(traceback.format_exc()) + return (mod, {}, {}, 0, ex) + + # Use ThreadPoolExecutor to load modules in parallel + with ThreadPoolExecutor(max_workers=4) as executor: + futures = [executor.submit(load_module_threaded, mod) + for mod in command_modules if mod not in BLOCKED_MODS] + + for future in futures: + mod, module_command_table, module_group_table, elapsed_time, error = future.result() + if error: + # Changing this error message requires updating CI script that checks for failed + # module loading. + from azure.cli.core import telemetry + logger.error("Error loading command module '%s': %s", mod, error) + telemetry.set_exception(exception=error, fault_type='module-load-error-' + mod, + summary='Error loading module: {}'.format(mod)) + logger.debug(traceback.format_exc()) + else: + for cmd in module_command_table.values(): + cmd.command_source = mod + self.command_table.update(module_command_table) + self.command_group_table.update(module_group_table) + + logger.debug(self.item_format_string, mod, elapsed_time, + len(module_group_table), len(module_command_table)) + count += 1 + cumulative_elapsed_time += elapsed_time + cumulative_group_count += len(module_group_table) + cumulative_command_count += len(module_command_table) + elapsed_time = timeit.default_timer() - start_time_wrapper + print(f"*** SUT operation *** took: {elapsed_time:.6f} seconds for {len(command_modules)} modules") # Summary line logger.debug(self.item_format_string, "Total ({})".format(count), cumulative_elapsed_time, diff --git a/src/azure-cli-core/azure/cli/core/commands/__init__.py b/src/azure-cli-core/azure/cli/core/commands/__init__.py index 696b6093f5d..4cbc168c70c 100644 --- a/src/azure-cli-core/azure/cli/core/commands/__init__.py +++ b/src/azure-cli-core/azure/cli/core/commands/__init__.py @@ -519,6 +519,7 @@ def execute(self, args): args = _pre_command_table_create(self.cli_ctx, args) self.cli_ctx.raise_event(EVENT_INVOKER_PRE_CMD_TBL_CREATE, args=args) + # @NOTE: below takes forever: self.commands_loader.load_command_table(args) self.cli_ctx.raise_event(EVENT_INVOKER_PRE_CMD_TBL_TRUNCATE, load_cmd_tbl_func=self.commands_loader.load_command_table, args=args) From 2f9343439e1a9b2bace8374359d30dda01cf428b Mon Sep 17 00:00:00 2001 From: Daniel Languiller Date: Wed, 10 Dec 2025 16:45:27 +1100 Subject: [PATCH 02/18] refactor: export to smaller methods --- src/azure-cli-core/azure/cli/core/__init__.py | 137 +++++++++++------- 1 file changed, 86 insertions(+), 51 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/__init__.py b/src/azure-cli-core/azure/cli/core/__init__.py index 32d4f4675b8..376c1696412 100644 --- a/src/azure-cli-core/azure/cli/core/__init__.py +++ b/src/azure-cli-core/azure/cli/core/__init__.py @@ -10,6 +10,8 @@ import sys import timeit from concurrent.futures import ThreadPoolExecutor +from dataclasses import dataclass +from typing import Dict, Any, Optional from knack.cli import CLI from knack.commands import CLICommandsLoader @@ -196,6 +198,15 @@ def _configure_style(self): format_styled_text.theme = theme +@dataclass +class ModuleLoadResult: + module_name: str + command_table: Dict[str, Any] + group_table: Dict[str, Any] + elapsed_time: float + error: Optional[Exception] = None + + class MainCommandsLoader(CLICommandsLoader): # Format string for pretty-print the command module table @@ -222,11 +233,11 @@ def load_command_table(self, args): import pkgutil import traceback from azure.cli.core.commands import ( - _load_module_command_loader, _load_extension_command_loader, BLOCKED_MODS, ExtensionCommandSource) + _load_extension_command_loader, ExtensionCommandSource) from azure.cli.core.extension import ( get_extensions, get_extension_path, get_extension_modname) from azure.cli.core.breaking_change import ( - import_core_breaking_changes, import_module_breaking_changes, import_extension_breaking_changes) + import_core_breaking_changes, import_extension_breaking_changes) def _update_command_table_from_modules(args, command_modules=None): """Loads command tables from modules and merge into the main command table. @@ -254,55 +265,11 @@ def _update_command_table_from_modules(args, command_modules=None): except ImportError as e: logger.warning(e) - count = 0 - cumulative_elapsed_time = 0 - cumulative_group_count = 0 - cumulative_command_count = 0 - logger.debug("Loading command modules:") - logger.debug(self.header_mod) - - print(f"*** Starting SUT***") - start_time_wrapper = timeit.default_timer() - - def load_module_threaded(mod): - try: - start_time = timeit.default_timer() - module_command_table, module_group_table = _load_module_command_loader(self, args, mod) - import_module_breaking_changes(mod) - elapsed_time = timeit.default_timer() - start_time - return (mod, module_command_table, module_group_table, elapsed_time, None) - except Exception as ex: # pylint: disable=broad-except - return (mod, {}, {}, 0, ex) - - # Use ThreadPoolExecutor to load modules in parallel - with ThreadPoolExecutor(max_workers=4) as executor: - futures = [executor.submit(load_module_threaded, mod) - for mod in command_modules if mod not in BLOCKED_MODS] - - for future in futures: - mod, module_command_table, module_group_table, elapsed_time, error = future.result() - if error: - # Changing this error message requires updating CI script that checks for failed - # module loading. - from azure.cli.core import telemetry - logger.error("Error loading command module '%s': %s", mod, error) - telemetry.set_exception(exception=error, fault_type='module-load-error-' + mod, - summary='Error loading module: {}'.format(mod)) - logger.debug(traceback.format_exc()) - else: - for cmd in module_command_table.values(): - cmd.command_source = mod - self.command_table.update(module_command_table) - self.command_group_table.update(module_group_table) - - logger.debug(self.item_format_string, mod, elapsed_time, - len(module_group_table), len(module_command_table)) - count += 1 - cumulative_elapsed_time += elapsed_time - cumulative_group_count += len(module_group_table) - cumulative_command_count += len(module_command_table) - elapsed_time = timeit.default_timer() - start_time_wrapper - print(f"*** SUT operation *** took: {elapsed_time:.6f} seconds for {len(command_modules)} modules") + results = self._load_modules_threaded(args, command_modules) + + # @TODO: export to own method: + count, cumulative_elapsed_time, cumulative_group_count, cumulative_command_count = \ + self._process_results_with_timing(results, command_modules) # Summary line logger.debug(self.item_format_string, "Total ({})".format(count), cumulative_elapsed_time, @@ -579,6 +546,74 @@ def load_arguments(self, command=None): self.extra_argument_registry.update(loader.extra_argument_registry) loader._update_command_definitions() # pylint: disable=protected-access + def _load_modules_threaded(self, args, command_modules): + """Load command modules using ThreadPoolExecutor for parallel processing.""" + from azure.cli.core.commands import _load_module_command_loader, BLOCKED_MODS + from azure.cli.core.breaking_change import import_module_breaking_changes + + def load_single_module(mod): + try: + start_time = timeit.default_timer() + module_command_table, module_group_table = _load_module_command_loader(self, args, mod) + import_module_breaking_changes(mod) + elapsed_time = timeit.default_timer() - start_time + return ModuleLoadResult(mod, module_command_table, module_group_table, elapsed_time) + except Exception as ex: # pylint: disable=broad-except + return ModuleLoadResult(mod, {}, {}, 0, ex) + + with ThreadPoolExecutor(max_workers=4) as executor: + futures = [executor.submit(load_single_module, mod) + for mod in command_modules if mod not in BLOCKED_MODS] + return [future.result() for future in futures] + + def _handle_module_load_error(self, result): + """Handle errors that occurred during module loading.""" + import traceback + from azure.cli.core import telemetry + + # Changing this error message requires updating CI script that checks for failed module loading. + logger.error("Error loading command module '%s': %s", result.module_name, result.error) + telemetry.set_exception(exception=result.error, + fault_type='module-load-error-' + result.module_name, + summary='Error loading module: {}'.format(result.module_name)) + logger.debug(traceback.format_exc()) + + def _process_successful_load(self, result): + """Process successfully loaded module results.""" + # Set command source for all commands in the module + for cmd in result.command_table.values(): + cmd.command_source = result.module_name + + # Update main command and group tables + self.command_table.update(result.command_table) + self.command_group_table.update(result.group_table) + + # Log the results + logger.debug(self.item_format_string, result.module_name, result.elapsed_time, + len(result.group_table), len(result.command_table)) + + def _process_results_with_timing(self, results, command_modules): + """Process pre-loaded module results with timing and progress reporting.""" + logger.debug("Loading command modules:") + logger.debug(self.header_mod) + + count = 0 + cumulative_elapsed_time = 0 + cumulative_group_count = 0 + cumulative_command_count = 0 + + for result in results: + if result.error: + self._handle_module_load_error(result) + else: + self._process_successful_load(result) + count += 1 + cumulative_elapsed_time += result.elapsed_time + cumulative_group_count += len(result.group_table) + cumulative_command_count += len(result.command_table) + + return count, cumulative_elapsed_time, cumulative_group_count, cumulative_command_count + class CommandIndex: From f6f2ae9ff763ef42916264f9cd5d6f69478bbe74 Mon Sep 17 00:00:00 2001 From: Daniel Languiller Date: Tue, 20 Jan 2026 11:40:24 +1100 Subject: [PATCH 03/18] rebase conflict --- src/azure-cli-core/azure/cli/core/__init__.py | 70 ++++++++++++------- 1 file changed, 44 insertions(+), 26 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/__init__.py b/src/azure-cli-core/azure/cli/core/__init__.py index 376c1696412..0c02ea04e71 100644 --- a/src/azure-cli-core/azure/cli/core/__init__.py +++ b/src/azure-cli-core/azure/cli/core/__init__.py @@ -10,7 +10,6 @@ import sys import timeit from concurrent.futures import ThreadPoolExecutor -from dataclasses import dataclass from typing import Dict, Any, Optional from knack.cli import CLI @@ -198,13 +197,13 @@ def _configure_style(self): format_styled_text.theme = theme -@dataclass class ModuleLoadResult: - module_name: str - command_table: Dict[str, Any] - group_table: Dict[str, Any] - elapsed_time: float - error: Optional[Exception] = None + def __init__(self, module_name, command_table, group_table, elapsed_time, error=None): + self.module_name = module_name + self.command_table = command_table + self.group_table = group_table + self.elapsed_time = elapsed_time + self.error = error class MainCommandsLoader(CLICommandsLoader): @@ -265,7 +264,7 @@ def _update_command_table_from_modules(args, command_modules=None): except ImportError as e: logger.warning(e) - results = self._load_modules_threaded(args, command_modules) + results = self._load_modules(args, command_modules) # @TODO: export to own method: count, cumulative_elapsed_time, cumulative_group_count, cumulative_command_count = \ @@ -546,25 +545,44 @@ def load_arguments(self, command=None): self.extra_argument_registry.update(loader.extra_argument_registry) loader._update_command_definitions() # pylint: disable=protected-access - def _load_modules_threaded(self, args, command_modules): - """Load command modules using ThreadPoolExecutor for parallel processing.""" - from azure.cli.core.commands import _load_module_command_loader, BLOCKED_MODS - from azure.cli.core.breaking_change import import_module_breaking_changes - - def load_single_module(mod): - try: - start_time = timeit.default_timer() - module_command_table, module_group_table = _load_module_command_loader(self, args, mod) - import_module_breaking_changes(mod) - elapsed_time = timeit.default_timer() - start_time - return ModuleLoadResult(mod, module_command_table, module_group_table, elapsed_time) - except Exception as ex: # pylint: disable=broad-except - return ModuleLoadResult(mod, {}, {}, 0, ex) - + def _load_modules(self, args, command_modules): + """Load command modules using ThreadPoolExecutor with timeout protection.""" + from azure.cli.core.commands import BLOCKED_MODS + import concurrent.futures + + results = [] with ThreadPoolExecutor(max_workers=4) as executor: - futures = [executor.submit(load_single_module, mod) - for mod in command_modules if mod not in BLOCKED_MODS] - return [future.result() for future in futures] + future_to_module = {executor.submit(self._load_single_module, mod, args): mod + for mod in command_modules if mod not in BLOCKED_MODS} + + for future in concurrent.futures.as_completed(future_to_module, timeout=60): + try: + # @NOTE: Timeout to counteract deadlocks, but how to test? + result = future.result(timeout=30) + results.append(result) + except concurrent.futures.TimeoutError: + mod = future_to_module[future] + logger.warning("Module '%s' load timeout", mod) + results.append(ModuleLoadResult(mod, {}, {}, 0, + Exception(f"Module '{mod}' load timeout"))) + except Exception as ex: + mod = future_to_module[future] + logger.warning("Module '%s' load failed: %s", mod, ex) + results.append(ModuleLoadResult(mod, {}, {}, 0, ex)) + + return results + + def _load_single_module(self, mod, args): + from azure.cli.core.breaking_change import import_module_breaking_changes + from azure.cli.core.commands import _load_module_command_loader + try: + start_time = timeit.default_timer() + module_command_table, module_group_table = _load_module_command_loader(self, args, mod) + import_module_breaking_changes(mod) + elapsed_time = timeit.default_timer() - start_time + return ModuleLoadResult(mod, module_command_table, module_group_table, elapsed_time) + except Exception as ex: # pylint: disable=broad-except + return ModuleLoadResult(mod, {}, {}, 0, ex) def _handle_module_load_error(self, result): """Handle errors that occurred during module loading.""" From e26cb0379974ae17dd3d64cac14fb8cf2d3c9de6 Mon Sep 17 00:00:00 2001 From: Daniel Languiller Date: Thu, 11 Dec 2025 14:58:33 +1100 Subject: [PATCH 04/18] refactor: use constants for magic numbers --- src/azure-cli-core/azure/cli/core/__init__.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/__init__.py b/src/azure-cli-core/azure/cli/core/__init__.py index 0c02ea04e71..be67cbd73fa 100644 --- a/src/azure-cli-core/azure/cli/core/__init__.py +++ b/src/azure-cli-core/azure/cli/core/__init__.py @@ -34,6 +34,8 @@ ALWAYS_LOADED_MODULES = [] # Extensions that will always be loaded if installed. They don't expose commands but hook into CLI core. ALWAYS_LOADED_EXTENSIONS = ['azext_ai_examples', 'azext_next'] +MODULE_LOAD_TIMEOUT_SECONDS = 30 +MAX_WORKER_THREAD_COUNT = 4 def _configure_knack(): @@ -551,18 +553,18 @@ def _load_modules(self, args, command_modules): import concurrent.futures results = [] - with ThreadPoolExecutor(max_workers=4) as executor: + with ThreadPoolExecutor(max_workers=MAX_WORKER_THREAD_COUNT) as executor: future_to_module = {executor.submit(self._load_single_module, mod, args): mod for mod in command_modules if mod not in BLOCKED_MODS} - for future in concurrent.futures.as_completed(future_to_module, timeout=60): + for future in future_to_module: try: - # @NOTE: Timeout to counteract deadlocks, but how to test? - result = future.result(timeout=30) + result = future.result(timeout=MODULE_LOAD_TIMEOUT_SECONDS) results.append(result) except concurrent.futures.TimeoutError: mod = future_to_module[future] - logger.warning("Module '%s' load timeout", mod) + logger.warning("Module '%s' load timeout after %s seconds", mod, MODULE_LOAD_TIMEOUT_SECONDS) + future.cancel() results.append(ModuleLoadResult(mod, {}, {}, 0, Exception(f"Module '{mod}' load timeout"))) except Exception as ex: From c284f7ff6f71ddc3b1acfd58ca7fe30dc9500485 Mon Sep 17 00:00:00 2001 From: Daniel Languiller Date: Thu, 11 Dec 2025 15:27:39 +1100 Subject: [PATCH 05/18] refactor: remove comments --- src/azure-cli-core/azure/cli/core/__init__.py | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/__init__.py b/src/azure-cli-core/azure/cli/core/__init__.py index be67cbd73fa..3ec496b46f0 100644 --- a/src/azure-cli-core/azure/cli/core/__init__.py +++ b/src/azure-cli-core/azure/cli/core/__init__.py @@ -268,7 +268,6 @@ def _update_command_table_from_modules(args, command_modules=None): results = self._load_modules(args, command_modules) - # @TODO: export to own method: count, cumulative_elapsed_time, cumulative_group_count, cumulative_command_count = \ self._process_results_with_timing(results, command_modules) # Summary line @@ -590,25 +589,21 @@ def _handle_module_load_error(self, result): """Handle errors that occurred during module loading.""" import traceback from azure.cli.core import telemetry - - # Changing this error message requires updating CI script that checks for failed module loading. + logger.error("Error loading command module '%s': %s", result.module_name, result.error) - telemetry.set_exception(exception=result.error, + telemetry.set_exception(exception=result.error, fault_type='module-load-error-' + result.module_name, summary='Error loading module: {}'.format(result.module_name)) logger.debug(traceback.format_exc()) def _process_successful_load(self, result): """Process successfully loaded module results.""" - # Set command source for all commands in the module for cmd in result.command_table.values(): cmd.command_source = result.module_name - - # Update main command and group tables + self.command_table.update(result.command_table) self.command_group_table.update(result.group_table) - - # Log the results + logger.debug(self.item_format_string, result.module_name, result.elapsed_time, len(result.group_table), len(result.command_table)) From de143342734bac96b66fe77af0c97cd2d8931a8c Mon Sep 17 00:00:00 2001 From: Daniel Languiller Date: Mon, 15 Dec 2025 13:21:49 +1100 Subject: [PATCH 06/18] test: add test for command table integrity --- .../tests/test_command_table_integrity.py | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 src/azure-cli-core/azure/cli/core/tests/test_command_table_integrity.py diff --git a/src/azure-cli-core/azure/cli/core/tests/test_command_table_integrity.py b/src/azure-cli-core/azure/cli/core/tests/test_command_table_integrity.py new file mode 100644 index 00000000000..28fa52dd355 --- /dev/null +++ b/src/azure-cli-core/azure/cli/core/tests/test_command_table_integrity.py @@ -0,0 +1,57 @@ +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- + +import unittest + +from azure.cli.core.mock import DummyCli +from azure.cli.core import MainCommandsLoader + + +class CommandTableIntegrityTest(unittest.TestCase): + + def setUp(self): + self.cli_ctx = DummyCli() + + def test_command_table_integrity(self): + """Test command table loading produces valid, complete results.""" + + # Load command table using current implementation + loader = MainCommandsLoader(self.cli_ctx) + loader.load_command_table([]) + + # Test invariants that should always hold: + + # 1. No corruption/duplicates + command_names = list(loader.command_table.keys()) + unique_command_names = set(command_names) + self.assertEqual(len(unique_command_names), len(command_names), "No duplicate commands") + + # 2. Core functionality exists (high-level groups that should always exist) + core_groups = ['vm', 'network', 'resource', 'account', 'group'] + existing_groups = {cmd.split()[0] for cmd in loader.command_table.keys() if ' ' in cmd} + missing_core = [group for group in core_groups if group not in existing_groups] + self.assertEqual(len(missing_core), 0, f"Missing core command groups: {missing_core}") + + # 3. Structural integrity + commands_without_source = [] + for cmd_name, cmd_obj in loader.command_table.items(): + if not hasattr(cmd_obj, 'command_source') or not cmd_obj.command_source: + commands_without_source.append(cmd_name) + + self.assertEqual(len(commands_without_source), 0, + f"Commands missing source: {commands_without_source[:5]}...") + + # 4. Basic sanity - we loaded SOMETHING + self.assertGreater(len(loader.command_table), 0, "Commands were loaded") + self.assertGreater(len(loader.command_group_table), 0, "Groups were loaded") + + # 5. Verify core groups are properly represented + found_core_groups = sorted(existing_groups & set(core_groups)) + self.assertGreaterEqual(len(found_core_groups), 3, + f"At least 3 core command groups should be present, found: {found_core_groups}") + + +if __name__ == '__main__': + unittest.main() From 285860514b72f0f5fda5cd5ca2a80b95ff30df5d Mon Sep 17 00:00:00 2001 From: Daniel Languiller Date: Mon, 15 Dec 2025 13:23:14 +1100 Subject: [PATCH 07/18] refactor: remove comments --- src/azure-cli-core/azure/cli/core/commands/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/azure-cli-core/azure/cli/core/commands/__init__.py b/src/azure-cli-core/azure/cli/core/commands/__init__.py index 4cbc168c70c..696b6093f5d 100644 --- a/src/azure-cli-core/azure/cli/core/commands/__init__.py +++ b/src/azure-cli-core/azure/cli/core/commands/__init__.py @@ -519,7 +519,6 @@ def execute(self, args): args = _pre_command_table_create(self.cli_ctx, args) self.cli_ctx.raise_event(EVENT_INVOKER_PRE_CMD_TBL_CREATE, args=args) - # @NOTE: below takes forever: self.commands_loader.load_command_table(args) self.cli_ctx.raise_event(EVENT_INVOKER_PRE_CMD_TBL_TRUNCATE, load_cmd_tbl_func=self.commands_loader.load_command_table, args=args) From f9fce95f2f7a76683824ff6366e26ffb62bc7f92 Mon Sep 17 00:00:00 2001 From: Daniel Languiller Date: Mon, 15 Dec 2025 15:17:11 +1100 Subject: [PATCH 08/18] refactor: restore launch.json --- .vscode/launch.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 88410318724..e7ed7a11353 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -23,7 +23,7 @@ "program": "${workspaceRoot}/src/azure-cli/azure/cli/__main__.py", "cwd": "${workspaceRoot}", "args": [ - "Version" + "--version" ], "console": "externalTerminal", }, From 869eacedff9a956a6128c8e5f023180de2313f79 Mon Sep 17 00:00:00 2001 From: Daniel Languiller Date: Mon, 15 Dec 2025 15:57:04 +1100 Subject: [PATCH 09/18] refactor: linting issues --- src/azure-cli-core/azure/cli/core/__init__.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/__init__.py b/src/azure-cli-core/azure/cli/core/__init__.py index 3ec496b46f0..865550058e3 100644 --- a/src/azure-cli-core/azure/cli/core/__init__.py +++ b/src/azure-cli-core/azure/cli/core/__init__.py @@ -9,8 +9,8 @@ import os import sys import timeit +import concurrent.futures from concurrent.futures import ThreadPoolExecutor -from typing import Dict, Any, Optional from knack.cli import CLI from knack.commands import CLICommandsLoader @@ -199,7 +199,7 @@ def _configure_style(self): format_styled_text.theme = theme -class ModuleLoadResult: +class ModuleLoadResult: # pylint: disable=too-few-public-methods def __init__(self, module_name, command_table, group_table, elapsed_time, error=None): self.module_name = module_name self.command_table = command_table @@ -269,7 +269,7 @@ def _update_command_table_from_modules(args, command_modules=None): results = self._load_modules(args, command_modules) count, cumulative_elapsed_time, cumulative_group_count, cumulative_command_count = \ - self._process_results_with_timing(results, command_modules) + self._process_results_with_timing(results) # Summary line logger.debug(self.item_format_string, "Total ({})".format(count), cumulative_elapsed_time, @@ -554,7 +554,7 @@ def _load_modules(self, args, command_modules): results = [] with ThreadPoolExecutor(max_workers=MAX_WORKER_THREAD_COUNT) as executor: future_to_module = {executor.submit(self._load_single_module, mod, args): mod - for mod in command_modules if mod not in BLOCKED_MODS} + for mod in command_modules if mod not in BLOCKED_MODS} for future in future_to_module: try: @@ -565,8 +565,8 @@ def _load_modules(self, args, command_modules): logger.warning("Module '%s' load timeout after %s seconds", mod, MODULE_LOAD_TIMEOUT_SECONDS) future.cancel() results.append(ModuleLoadResult(mod, {}, {}, 0, - Exception(f"Module '{mod}' load timeout"))) - except Exception as ex: + Exception(f"Module '{mod}' load timeout"))) + except (ImportError, AttributeError, TypeError, ValueError) as ex: mod = future_to_module[future] logger.warning("Module '%s' load failed: %s", mod, ex) results.append(ModuleLoadResult(mod, {}, {}, 0, ex)) @@ -605,9 +605,9 @@ def _process_successful_load(self, result): self.command_group_table.update(result.group_table) logger.debug(self.item_format_string, result.module_name, result.elapsed_time, - len(result.group_table), len(result.command_table)) + len(result.group_table), len(result.command_table)) - def _process_results_with_timing(self, results, command_modules): + def _process_results_with_timing(self, results): """Process pre-loaded module results with timing and progress reporting.""" logger.debug("Loading command modules:") logger.debug(self.header_mod) From 66844ab9f8be77c71ac89bb5c7e731395f59d5bf Mon Sep 17 00:00:00 2001 From: Daniel Languiller Date: Mon, 15 Dec 2025 15:59:51 +1100 Subject: [PATCH 10/18] refactor: flake issues --- src/azure-cli-core/azure/cli/core/__init__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/__init__.py b/src/azure-cli-core/azure/cli/core/__init__.py index 865550058e3..f2e7362ae75 100644 --- a/src/azure-cli-core/azure/cli/core/__init__.py +++ b/src/azure-cli-core/azure/cli/core/__init__.py @@ -234,7 +234,7 @@ def load_command_table(self, args): import pkgutil import traceback from azure.cli.core.commands import ( - _load_extension_command_loader, ExtensionCommandSource) + _load_extension_command_loader, ExtensionCommandSource) from azure.cli.core.extension import ( get_extensions, get_extension_path, get_extension_modname) from azure.cli.core.breaking_change import ( @@ -565,7 +565,7 @@ def _load_modules(self, args, command_modules): logger.warning("Module '%s' load timeout after %s seconds", mod, MODULE_LOAD_TIMEOUT_SECONDS) future.cancel() results.append(ModuleLoadResult(mod, {}, {}, 0, - Exception(f"Module '{mod}' load timeout"))) + Exception(f"Module '{mod}' load timeout"))) except (ImportError, AttributeError, TypeError, ValueError) as ex: mod = future_to_module[future] logger.warning("Module '%s' load failed: %s", mod, ex) @@ -592,8 +592,8 @@ def _handle_module_load_error(self, result): logger.error("Error loading command module '%s': %s", result.module_name, result.error) telemetry.set_exception(exception=result.error, - fault_type='module-load-error-' + result.module_name, - summary='Error loading module: {}'.format(result.module_name)) + fault_type='module-load-error-' + result.module_name, + summary='Error loading module: {}'.format(result.module_name)) logger.debug(traceback.format_exc()) def _process_successful_load(self, result): From 782916bd13f1c7cdca4046ebaac3cf8be58ca2c4 Mon Sep 17 00:00:00 2001 From: Daniel Languiller Date: Mon, 15 Dec 2025 16:05:40 +1100 Subject: [PATCH 11/18] refactor: flake issues --- src/azure-cli-core/azure/cli/core/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/azure-cli-core/azure/cli/core/__init__.py b/src/azure-cli-core/azure/cli/core/__init__.py index f2e7362ae75..be023eb1b0b 100644 --- a/src/azure-cli-core/azure/cli/core/__init__.py +++ b/src/azure-cli-core/azure/cli/core/__init__.py @@ -565,7 +565,7 @@ def _load_modules(self, args, command_modules): logger.warning("Module '%s' load timeout after %s seconds", mod, MODULE_LOAD_TIMEOUT_SECONDS) future.cancel() results.append(ModuleLoadResult(mod, {}, {}, 0, - Exception(f"Module '{mod}' load timeout"))) + Exception(f"Module '{mod}' load timeout"))) except (ImportError, AttributeError, TypeError, ValueError) as ex: mod = future_to_module[future] logger.warning("Module '%s' load failed: %s", mod, ex) From 272fdfa2b3edf695d5be48060a314b5fa036522a Mon Sep 17 00:00:00 2001 From: Daniel Languiller Date: Mon, 15 Dec 2025 17:10:33 +1100 Subject: [PATCH 12/18] refactor: fix lint issue (imports) --- src/azure-cli-core/azure/cli/core/__init__.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/__init__.py b/src/azure-cli-core/azure/cli/core/__init__.py index be023eb1b0b..c5dcf20b2e2 100644 --- a/src/azure-cli-core/azure/cli/core/__init__.py +++ b/src/azure-cli-core/azure/cli/core/__init__.py @@ -9,8 +9,7 @@ import os import sys import timeit -import concurrent.futures -from concurrent.futures import ThreadPoolExecutor +from concurrent.futures import ThreadPoolExecutor, TimeoutError from knack.cli import CLI from knack.commands import CLICommandsLoader @@ -549,7 +548,6 @@ def load_arguments(self, command=None): def _load_modules(self, args, command_modules): """Load command modules using ThreadPoolExecutor with timeout protection.""" from azure.cli.core.commands import BLOCKED_MODS - import concurrent.futures results = [] with ThreadPoolExecutor(max_workers=MAX_WORKER_THREAD_COUNT) as executor: @@ -560,7 +558,7 @@ def _load_modules(self, args, command_modules): try: result = future.result(timeout=MODULE_LOAD_TIMEOUT_SECONDS) results.append(result) - except concurrent.futures.TimeoutError: + except TimeoutError: mod = future_to_module[future] logger.warning("Module '%s' load timeout after %s seconds", mod, MODULE_LOAD_TIMEOUT_SECONDS) future.cancel() From 1e1ac9fe191c4d287d5b0b24d94d742adb4b5f9d Mon Sep 17 00:00:00 2001 From: Daniel Languiller Date: Tue, 16 Dec 2025 09:42:58 +1100 Subject: [PATCH 13/18] refactor: concurrent.futures Timeout import --- src/azure-cli-core/azure/cli/core/__init__.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/__init__.py b/src/azure-cli-core/azure/cli/core/__init__.py index c5dcf20b2e2..09dfb0fc2fd 100644 --- a/src/azure-cli-core/azure/cli/core/__init__.py +++ b/src/azure-cli-core/azure/cli/core/__init__.py @@ -9,7 +9,8 @@ import os import sys import timeit -from concurrent.futures import ThreadPoolExecutor, TimeoutError +import concurrent.futures +from concurrent.futures import ThreadPoolExecutor from knack.cli import CLI from knack.commands import CLICommandsLoader @@ -558,7 +559,7 @@ def _load_modules(self, args, command_modules): try: result = future.result(timeout=MODULE_LOAD_TIMEOUT_SECONDS) results.append(result) - except TimeoutError: + except concurrent.futures.TimeoutError: mod = future_to_module[future] logger.warning("Module '%s' load timeout after %s seconds", mod, MODULE_LOAD_TIMEOUT_SECONDS) future.cancel() From b52df25b1da0808b837c81b181536797ef6e3d31 Mon Sep 17 00:00:00 2001 From: Daniel Languiller Date: Thu, 18 Dec 2025 12:59:42 +1100 Subject: [PATCH 14/18] fix: address co-pilot suggestions --- src/azure-cli-core/azure/cli/core/__init__.py | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/__init__.py b/src/azure-cli-core/azure/cli/core/__init__.py index 09dfb0fc2fd..e9a78770eb6 100644 --- a/src/azure-cli-core/azure/cli/core/__init__.py +++ b/src/azure-cli-core/azure/cli/core/__init__.py @@ -34,7 +34,9 @@ ALWAYS_LOADED_MODULES = [] # Extensions that will always be loaded if installed. They don't expose commands but hook into CLI core. ALWAYS_LOADED_EXTENSIONS = ['azext_ai_examples', 'azext_next'] +# Timeout (in seconds) for loading a single module. Acts as a safety valve to prevent indefinite hangs MODULE_LOAD_TIMEOUT_SECONDS = 30 +# Maximum number of worker threads for parallel module loading. MAX_WORKER_THREAD_COUNT = 4 @@ -200,12 +202,13 @@ def _configure_style(self): class ModuleLoadResult: # pylint: disable=too-few-public-methods - def __init__(self, module_name, command_table, group_table, elapsed_time, error=None): + def __init__(self, module_name, command_table, group_table, elapsed_time, error=None, traceback_str=None): self.module_name = module_name self.command_table = command_table self.group_table = group_table self.elapsed_time = elapsed_time self.error = error + self.traceback_str = traceback_str class MainCommandsLoader(CLICommandsLoader): @@ -555,26 +558,30 @@ def _load_modules(self, args, command_modules): future_to_module = {executor.submit(self._load_single_module, mod, args): mod for mod in command_modules if mod not in BLOCKED_MODS} - for future in future_to_module: + for future in concurrent.futures.as_completed(future_to_module): try: result = future.result(timeout=MODULE_LOAD_TIMEOUT_SECONDS) results.append(result) except concurrent.futures.TimeoutError: mod = future_to_module[future] logger.warning("Module '%s' load timeout after %s seconds", mod, MODULE_LOAD_TIMEOUT_SECONDS) - future.cancel() results.append(ModuleLoadResult(mod, {}, {}, 0, Exception(f"Module '{mod}' load timeout"))) except (ImportError, AttributeError, TypeError, ValueError) as ex: mod = future_to_module[future] logger.warning("Module '%s' load failed: %s", mod, ex) results.append(ModuleLoadResult(mod, {}, {}, 0, ex)) + except Exception as ex: + mod = future_to_module[future] + logger.warning("Module '%s' load failed with unexpected exception: %s", mod, ex) + results.append(ModuleLoadResult(mod, {}, {}, 0, ex)) return results def _load_single_module(self, mod, args): from azure.cli.core.breaking_change import import_module_breaking_changes from azure.cli.core.commands import _load_module_command_loader + import traceback try: start_time = timeit.default_timer() module_command_table, module_group_table = _load_module_command_loader(self, args, mod) @@ -582,18 +589,19 @@ def _load_single_module(self, mod, args): elapsed_time = timeit.default_timer() - start_time return ModuleLoadResult(mod, module_command_table, module_group_table, elapsed_time) except Exception as ex: # pylint: disable=broad-except - return ModuleLoadResult(mod, {}, {}, 0, ex) + tb_str = traceback.format_exc() + return ModuleLoadResult(mod, {}, {}, 0, ex, tb_str) def _handle_module_load_error(self, result): """Handle errors that occurred during module loading.""" - import traceback from azure.cli.core import telemetry logger.error("Error loading command module '%s': %s", result.module_name, result.error) telemetry.set_exception(exception=result.error, fault_type='module-load-error-' + result.module_name, summary='Error loading module: {}'.format(result.module_name)) - logger.debug(traceback.format_exc()) + if result.traceback_str: + logger.debug(result.traceback_str) def _process_successful_load(self, result): """Process successfully loaded module results.""" From e28cd4e2079df6edf385e4d45566b04e493735cb Mon Sep 17 00:00:00 2001 From: Daniel Languiller Date: Thu, 18 Dec 2025 13:32:43 +1100 Subject: [PATCH 15/18] fix: address lint error --- src/azure-cli-core/azure/cli/core/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/azure-cli-core/azure/cli/core/__init__.py b/src/azure-cli-core/azure/cli/core/__init__.py index e9a78770eb6..0d465a249bd 100644 --- a/src/azure-cli-core/azure/cli/core/__init__.py +++ b/src/azure-cli-core/azure/cli/core/__init__.py @@ -571,7 +571,7 @@ def _load_modules(self, args, command_modules): mod = future_to_module[future] logger.warning("Module '%s' load failed: %s", mod, ex) results.append(ModuleLoadResult(mod, {}, {}, 0, ex)) - except Exception as ex: + except Exception as ex: # pylint: disable=broad-exception-caught mod = future_to_module[future] logger.warning("Module '%s' load failed with unexpected exception: %s", mod, ex) results.append(ModuleLoadResult(mod, {}, {}, 0, ex)) From 81ced7db794ecfff0cd6065133431124d3a4e4d7 Mon Sep 17 00:00:00 2001 From: Daniel Languiller Date: Mon, 22 Dec 2025 15:44:20 +1100 Subject: [PATCH 16/18] feature: address threading race conditions concerns --- src/azure-cli-core/azure/cli/core/__init__.py | 15 +++++++++++---- .../azure/cli/core/commands/__init__.py | 13 ++++--------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/__init__.py b/src/azure-cli-core/azure/cli/core/__init__.py index 0d465a249bd..7d03dd11ac2 100644 --- a/src/azure-cli-core/azure/cli/core/__init__.py +++ b/src/azure-cli-core/azure/cli/core/__init__.py @@ -202,13 +202,14 @@ def _configure_style(self): class ModuleLoadResult: # pylint: disable=too-few-public-methods - def __init__(self, module_name, command_table, group_table, elapsed_time, error=None, traceback_str=None): + def __init__(self, module_name, command_table, group_table, elapsed_time, error=None, traceback_str=None, command_loader=None): self.module_name = module_name self.command_table = command_table self.group_table = group_table self.elapsed_time = elapsed_time self.error = error self.traceback_str = traceback_str + self.command_loader = command_loader class MainCommandsLoader(CLICommandsLoader): @@ -346,7 +347,7 @@ def _filter_modname(extensions): # from an extension requires this map to be up-to-date. # self._mod_to_ext_map[ext_mod] = ext_name start_time = timeit.default_timer() - extension_command_table, extension_group_table = \ + extension_command_table, extension_group_table, _ = \ _load_extension_command_loader(self, args, ext_mod) import_extension_breaking_changes(ext_mod) @@ -584,10 +585,10 @@ def _load_single_module(self, mod, args): import traceback try: start_time = timeit.default_timer() - module_command_table, module_group_table = _load_module_command_loader(self, args, mod) + module_command_table, module_group_table, command_loader = _load_module_command_loader(self, args, mod) import_module_breaking_changes(mod) elapsed_time = timeit.default_timer() - start_time - return ModuleLoadResult(mod, module_command_table, module_group_table, elapsed_time) + return ModuleLoadResult(mod, module_command_table, module_group_table, elapsed_time, command_loader=command_loader) except Exception as ex: # pylint: disable=broad-except tb_str = traceback.format_exc() return ModuleLoadResult(mod, {}, {}, 0, ex, tb_str) @@ -605,6 +606,12 @@ def _handle_module_load_error(self, result): def _process_successful_load(self, result): """Process successfully loaded module results.""" + if result.command_loader: + self.loaders.append(result.command_loader) + + for cmd in result.command_table: + self.cmd_to_loader_map[cmd] = [result.command_loader] + for cmd in result.command_table.values(): cmd.command_source = result.module_name diff --git a/src/azure-cli-core/azure/cli/core/commands/__init__.py b/src/azure-cli-core/azure/cli/core/commands/__init__.py index 696b6093f5d..1764fbdf25e 100644 --- a/src/azure-cli-core/azure/cli/core/commands/__init__.py +++ b/src/azure-cli-core/azure/cli/core/commands/__init__.py @@ -1134,22 +1134,17 @@ def _load_command_loader(loader, args, name, prefix): logger.debug("Module '%s' is missing `get_command_loader` entry.", name) command_table = {} + command_loader = None if loader_cls: command_loader = loader_cls(cli_ctx=loader.cli_ctx) - loader.loaders.append(command_loader) # This will be used by interactive if command_loader.supported_resource_type(): command_table = command_loader.load_command_table(args) - if command_table: - for cmd in list(command_table.keys()): - # TODO: If desired to for extension to patch module, this can be uncommented - # if loader.cmd_to_loader_map.get(cmd): - # loader.cmd_to_loader_map[cmd].append(command_loader) - # else: - loader.cmd_to_loader_map[cmd] = [command_loader] else: logger.debug("Module '%s' is missing `COMMAND_LOADER_CLS` entry.", name) - return command_table, command_loader.command_group_table + + group_table = command_loader.command_group_table if command_loader else {} + return command_table, group_table, command_loader def _load_extension_command_loader(loader, args, ext): From 8b414bdb7af5bd1851d4d4a49ef45cf8b8f2f960 Mon Sep 17 00:00:00 2001 From: Daniel Languiller Date: Mon, 22 Dec 2025 16:18:36 +1100 Subject: [PATCH 17/18] fix: fix test mocks to return correct params --- .../azure/cli/core/tests/test_command_registration.py | 2 +- src/azure-cli-core/azure/cli/core/tests/test_parser.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/tests/test_command_registration.py b/src/azure-cli-core/azure/cli/core/tests/test_command_registration.py index 21d03d47b5e..41cf1ea1af4 100644 --- a/src/azure-cli-core/azure/cli/core/tests/test_command_registration.py +++ b/src/azure-cli-core/azure/cli/core/tests/test_command_registration.py @@ -230,7 +230,7 @@ def load_command_table(self, args): if command_table: module_command_table.update(command_table) loader.loaders.append(command_loader) # this will be used later by the load_arguments method - return module_command_table, command_loader.command_group_table + return module_command_table, command_loader.command_group_table, command_loader expected_command_index = {'hello': ['azure.cli.command_modules.hello', 'azext_hello2', 'azext_hello1'], 'extra': ['azure.cli.command_modules.extra']} diff --git a/src/azure-cli-core/azure/cli/core/tests/test_parser.py b/src/azure-cli-core/azure/cli/core/tests/test_parser.py index bff9c8ddc62..d584aafe383 100644 --- a/src/azure-cli-core/azure/cli/core/tests/test_parser.py +++ b/src/azure-cli-core/azure/cli/core/tests/test_parser.py @@ -188,7 +188,7 @@ def load_command_table(self, args): if command_table: module_command_table.update(command_table) loader.loaders.append(command_loader) # this will be used later by the load_arguments method - return module_command_table, command_loader.command_group_table + return module_command_table, command_loader.command_group_table, command_loader @mock.patch('importlib.import_module', _mock_import_lib) @mock.patch('pkgutil.iter_modules', _mock_iter_modules) From 220f6729ad0872a2b05755d1050f2b5e24f9f3a8 Mon Sep 17 00:00:00 2001 From: Daniel Languiller Date: Tue, 20 Jan 2026 12:28:28 +1100 Subject: [PATCH 18/18] fix: fix testing assertion from list to set --- .../azure/cli/core/tests/test_command_registration.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/tests/test_command_registration.py b/src/azure-cli-core/azure/cli/core/tests/test_command_registration.py index 41cf1ea1af4..174762624c0 100644 --- a/src/azure-cli-core/azure/cli/core/tests/test_command_registration.py +++ b/src/azure-cli-core/azure/cli/core/tests/test_command_registration.py @@ -432,12 +432,12 @@ def test_command_index_positional_argument(self): # Test command index is built for command with positional argument cmd_tbl = loader.load_command_table(["extra", "extra", "positional_argument"]) self.assertDictEqual(INDEX[CommandIndex._COMMAND_INDEX], self.expected_command_index) - self.assertEqual(list(cmd_tbl), ['hello mod-only', 'hello overridden', 'extra final', 'hello ext-only']) + self.assertSetEqual(set(cmd_tbl), {'hello mod-only', 'hello overridden', 'extra final', 'hello ext-only'}) # Test command index is used by command with positional argument cmd_tbl = loader.load_command_table(["hello", "mod-only", "positional_argument"]) self.assertDictEqual(INDEX[CommandIndex._COMMAND_INDEX], self.expected_command_index) - self.assertEqual(list(cmd_tbl), ['hello mod-only', 'hello overridden', 'hello ext-only']) + self.assertSetEqual(set(cmd_tbl), {'hello mod-only', 'hello overridden', 'hello ext-only'}) # Test command index is used by command with positional argument cmd_tbl = loader.load_command_table(["extra", "final", "positional_argument2"])