Skip to content

Conversation

@fengzhou-msft
Copy link
Member

@fengzhou-msft fengzhou-msft commented Mar 26, 2021

Description

Move the logic of dynamic extension install to a new fucntion _check_value_in_extensions. It will exit if the parser error is caused by an extension not installed, and simplified error handling code below by removing the not caused_by_extension_not_installed check based on this change.

This PR also adds the check whether the latest version of the extension is already installed to avoid infinite loop in cases like #17497

Testing Guide

History Notes

[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change.
[Component Name 2] az command b: Add some customer-facing feature.


This checklist is used to make sure that common guidelines for a pull request are followed.

Copy link
Contributor

@houk-ms houk-ms left a comment

Choose a reason for hiding this comment

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

good job

Comment on lines +542 to +338
recommender = CommandRecommender(*command_arguments, error_msg, cli_ctx)
recommender.set_help_examples(self.get_examples(command_name_inferred))
recommendations = recommender.provide_recommendations()
if recommendations:
az_error.set_aladdin_recommendation(recommendations)

# remind user to check extensions if we can not find a command to recommend
if isinstance(az_error, CommandNotFoundError) \
and not az_error.recommendations and self.prog == 'az' \
and use_dynamic_install == 'no':
az_error.set_recommendation(EXTENSION_REFERENCE)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can move these lines to another file too.

@fengzhou-msft fengzhou-msft force-pushed the refactor_dynamic_install branch from 76ef8ec to 871d1b7 Compare April 1, 2021 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants