-
Notifications
You must be signed in to change notification settings - Fork 3k
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.
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
{Core} Fix #14175: Load ALWAYS_LOADED_EXTENSIONS correctly #14180
Conversation
if command_str in self.command_group_table: | ||
logger.debug("Found a match in the command group table for '%s'", command_str) | ||
return self.command_table |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Separate the debug log for found in command table and found in the command group table.
@@ -153,7 +157,7 @@ def save_local_context(self, parsed_args, argument_definitions, specified_argume | |||
class MainCommandsLoader(CLICommandsLoader): | |||
|
|||
# Format string for pretty-print the command module table | |||
header_mod = "%-20s %10s %9s %9s" % ("Extension", "Load Time", "Groups", "Commands") | |||
header_mod = "%-20s %10s %9s %9s" % ("Name", "Load Time", "Groups", "Commands") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix an incorrect naming.
# Filter the extensions according to the index | ||
if ext_mod in extension_modname: | ||
filtered_extensions.append(ext) | ||
extension_modname.remove(ext_mod) | ||
if extension_modname: | ||
logger.debug("These extensions are not installed and will be skipped: %s", extension_modname) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will show something like
These extensions are not installed and will be skipped: ['azext_ai_examples', 'azext_ai_did_you_mean_this']
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
add to S172 |
Description
Fix #14175: ai-did-you-mean-this extension is not loaded after a first run
Currently, if no extension corresponds to a command like
az account
in Command Index:_update_command_table_from_extensions
won't be called, thus leftALWAYS_LOADED_EXTENSIONS
ignored.This PR fixes this issue by always calling
_update_command_table_from_modules
and_update_command_table_from_extensions
.Testing Guide