Skip to content

Commit

Permalink
Add reasonable warning when command on wheel extentions without --inc…
Browse files Browse the repository at this point in the history
…lude-whl-extensions (#133)

* Optimize help message of linter
* Add another warning for command runing on a wheel installed extension without --include-whl-extensions
* Clean code
* Partially fix Azure/azure-cli#11140
* Partially fix Azure/azure-cli-extensions#1078
  • Loading branch information
Jianhui Harold authored Nov 12, 2019
1 parent d25312d commit c55ab02
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 23 deletions.
11 changes: 9 additions & 2 deletions azdev/params.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ class Flag(object):
# pylint: disable=too-many-statements
def load_arguments(self, _):

modules_type = CLIArgumentType(nargs='*', help="Space-separated list of modules or extensions to check. Omit to check all or use 'CLI' or 'EXT' to check only CLI modules or extensions respectively.")
modules_type = CLIArgumentType(nargs='*',
help="Space-separated list of modules or extensions (dev mode) to check. "
"Use 'CLI' to check built-in modules or 'EXT' to check extensions. "
"Omit to check all. ")

with ArgumentsContext(self, '') as c:
c.argument('private', action='store_true', help='Target the private repo.')
Expand Down Expand Up @@ -65,12 +68,16 @@ def load_arguments(self, _):
with ArgumentsContext(self, 'cli update-setup') as c:
c.argument('pin', action='store_true', help='Pin the module versions in azure-cli\'s setup.py file.')

# region linter
with ArgumentsContext(self, 'linter') as c:
c.positional('modules', modules_type)
c.argument('rules', options_list=['--rules', '-r'], nargs='+', help='Space-separated list of rules to run. Omit to run all rules.')
c.argument('rule_types', options_list=['--rule-types', '-t'], nargs='+', choices=['params', 'commands', 'command_groups', 'help_entries'], help='Space-separated list of rule types to run. Omit to run all.')
c.argument('ci_exclusions', action='store_true', help='Force application of CI exclusions list when run locally.')
c.argument('include_whl_extensions', action='store_true', help='Allow running the linter on non-dev extensions (those installed using `az extension add ...`).')
c.argument('include_whl_extensions',
action='store_true',
help="Allow running linter on extensions installed by `az extension add`.")
# endregion

with ArgumentsContext(self, 'perf') as c:
c.argument('runs', type=int, help='Number of runs to average performance over.')
Expand Down
40 changes: 19 additions & 21 deletions azdev/utilities/path.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ def make_dirs(path):
def get_name_index(invert=False, include_whl_extensions=False):
""" Returns a dictionary containing the long and short names of modules and extensions is {SHORT:LONG} format or
{LONG:SHORT} format when invert=True. """
from azure.cli.core.extension import EXTENSIONS_DIR # pylint: disable=import-error

table = {}
cli_repo_path = get_cli_repo_path()
Expand All @@ -135,7 +136,6 @@ def get_name_index(invert=False, include_whl_extensions=False):
ext_paths = [x for x in find_files(ext_repo_paths, '*.*-info') if 'site-packages' not in x]
whl_ext_paths = []
if include_whl_extensions:
from azure.cli.core.extension import EXTENSIONS_DIR # pylint: disable=import-error
whl_ext_paths = [x for x in find_files(EXTENSIONS_DIR, '*.*-info') if 'site-packages' not in x]

def _update_table(paths, key):
Expand Down Expand Up @@ -190,6 +190,7 @@ def get_path_table(include_only=None, include_whl_extensions=False):
}
}
"""
from azure.cli.core.extension import EXTENSIONS_DIR # pylint: disable=import-error

# determine whether the call will filter or return all
if isinstance(include_only, str):
Expand All @@ -208,40 +209,30 @@ def get_path_table(include_only=None, include_whl_extensions=False):
modules_paths = glob(paths)
core_paths = glob(os.path.normcase(os.path.join(cli_repo_path, 'src', '*', 'setup.py')))
ext_paths = [x for x in find_files(ext_repo_paths, '*.*-info') if 'site-packages' not in x]
whl_ext_paths = []
if include_whl_extensions:
from azure.cli.core.extension import EXTENSIONS_DIR # pylint: disable=import-error
whl_ext_paths = [x for x in find_files(EXTENSIONS_DIR, '*.*-info') if 'site-packages' not in x]
whl_ext_paths = [x for x in find_files(EXTENSIONS_DIR, '*.*-info') if 'site-packages' not in x]

def _update_table(paths, key):
def _update_table(package_paths, key):
if key not in table:
table[key] = {}
folder = None
long_name = None
short_name = None
for path in paths:

for path in package_paths:
folder = os.path.dirname(path)
base_name = os.path.basename(folder)
# determine long-names

if key == 'ext':
short_name = base_name
for item in os.listdir(folder):
if item.startswith(EXTENSION_PREFIX):
long_name = item
break
long_name = next((item for item in os.listdir(folder) if item.startswith(EXTENSION_PREFIX)), None)
elif base_name.startswith(COMMAND_MODULE_PREFIX):
long_name = base_name
short_name = base_name.replace(COMMAND_MODULE_PREFIX, '') or '__main__'
long_name = base_name
else:
short_name = base_name
long_name = '{}{}'.format(COMMAND_MODULE_PREFIX, base_name)

if get_all:
table[key][long_name if key == 'ext' else short_name] = folder
continue
elif not include_only:
# nothing left to filter
return
return # nothing left to filter
else:
# check and update filter
if short_name in include_only:
Expand All @@ -256,9 +247,16 @@ def _update_table(paths, key):
_update_table(modules_paths, 'mod')
_update_table(core_paths, 'core')
_update_table(ext_paths, 'ext')
_update_table(whl_ext_paths, 'ext')
if include_whl_extensions:
_update_table(whl_ext_paths, 'ext')

if include_only:
raise CLIError('unrecognized names: {}'.format(' '.join(include_only)))
whl_extensions = [mod for whl_ext_path in whl_ext_paths for mod in include_only if mod in whl_ext_path]
if whl_extensions:
err = 'extension(s): [ {} ] installed from a wheel may need --include-whl-extensions option'.format(
', '.join(whl_extensions))
raise CLIError(err)

raise CLIError('unrecognized modules: [ {} ]'.format(', '.join(include_only)))

return table

0 comments on commit c55ab02

Please sign in to comment.