[Extension] Support installing extensions in a system directory #12856

fengzhou-msft · 2020-04-02T15:04:47Z

Description of PR (Mandatory)
(Why this PR? What is changed? What is the effect? etc. A high-quality description can accelerate the review process)
Resolve #9831.
Currently users can set AZURE_EXTENSION_DIR env variable to install extensions in a customized path. But this has a limit to install all extensions in one directory.

This PR adds an option --system to az extension add. So users can choose to install an extension in either a user path or a system path, but not able to install the same extension in both paths. az will search and load all the extensions in both paths. On Windows, users need to open the shell as Administrator to install extensions with --system.

The system directory is also configurable by env var AZURE_EXTENSION_SYS_DIR. The default is the azure-cli-extensions folder under the lib path of the python that az is using.
On windows with MSI, the directory is:
C:\Program Files (x86)\Microsoft SDKs\Azure\CLI2\lib\site-packages\azure-cli-extensions
On Ubuntu:
/opt/az/lib/python3.6/site-packages/azure-cli-extensions
On CentOS:
/usr/lib/python3.6/site-packages/azure-cli-extensions
On MacOS:
/usr/local/Cellar/azure-cli/2.3.1/libexec/lib/python3.8/site-packages/azure-cli-extensions

This PR also reduces the invoke of get_extension when possible since it will load all extensions.

Testing Guide
(Example commands with explanations)
az extension add -n extension-name --system
az extension show -n extension-name

History Notes:
(Fill in the following template if multiple notes are needed, otherwise PR title will be used for history note.)

[Extension] az extension add: Add --system to enable installing extensions in a system path

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

The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.

fengzhou-msft · 2020-04-02T15:05:54Z

src/azure-cli-core/azure/cli/core/extension/__init__.py

                                                                                          'cliextensions')
 DEV_EXTENSION_SOURCES = _DEV_EXTENSION_SOURCES.split(',') if _DEV_EXTENSION_SOURCES else []
-EXTENSIONS_SYS_DIR = os.path.join(get_python_lib(), 'azure-cli-extensions') if sys.platform.startswith('linux') else ""
+EXTENSIONS_SYS_DIR = os.path.expanduser(_CUSTOM_EXT_SYS_DIR) if _CUSTOM_EXT_SYS_DIR else os.path.join(get_python_lib(), 'azure-cli-extensions')


Need to test if this also works on Windows and Mac.

On windows, users need to open the shell as administrator. I have added message when users encounter the permission error.

run as admin only required when user set customized install dir right? if yes, then less concern since no change to existing behavoir

yes, not affecting default extension installation.

fengzhou-msft · 2020-04-02T15:07:50Z

src/azure-cli-core/azure/cli/core/extension/operations.py

    extension_name = _add_whl_ext(cmd=cmd, source=source, ext_sha256=ext_sha256,
-                                  pip_extra_index_urls=pip_extra_index_urls, pip_proxy=pip_proxy)
-    _augment_telemetry_with_ext_info(extension_name)
+                                  pip_extra_index_urls=pip_extra_index_urls, pip_proxy=pip_proxy, system=system)
+    ext = get_extension(extension_name)
+    _augment_telemetry_with_ext_info(extension_name, ext)
    try:
-        if extension_name and get_extension(extension_name).experimental:
+        if extension_name and ext.experimental:
            logger.warning("The installed extension '%s' is experimental and not covered by customer support. "
                           "Please use with discretion.", extension_name)
-        elif extension_name and get_extension(extension_name).preview:
+        elif extension_name and ext.preview:
            logger.warning("The installed extension '%s' is in preview.", extension_name)


get_extension calls get_extensions to load all extension, then fetch the extension with the name, we should avoid calling it mulitple times.

yonzhan · 2020-04-11T11:57:49Z

add to S168

…zure-cli into install_system_extension

arrownj · 2020-04-15T10:11:57Z

src/azure-cli-core/azure/cli/core/extension/__init__.py

-    return os.path.join(EXTENSIONS_DIR, ext_name)
+    ext_sys_path = os.path.join(EXTENSIONS_SYS_DIR, ext_name)
+    ext_path = os.path.join(EXTENSIONS_DIR, ext_name)
+    return ext_path if os.path.isdir(ext_path) else (


add os.path.isdir(ext_path) check here, is it because previous code may case potential bug ?

Previous code use get_extension_path both before install when this path does not exist and when loading extensions. I splitted it to use the below build_extension_path for building the path before install, and to use this get_extension_path only for loading extensions. So we can add the isdir check here now.

arrownj · 2020-04-15T10:20:47Z

src/azure-cli-core/azure/cli/core/extension/operations.py

    try:
-        if extension_name and get_extension(extension_name).experimental:
+        ext = get_extension(extension_name)
+        _augment_telemetry_with_ext_info(extension_name, ext)


move telemetry part into the try/except, so if get_extension throw exception, it will not be executed, right ?

Yes. get_extension was also called in _augment_telemetry_with_ext_info in previous code and it will be catched inside. And I think it's the same flow.

src/azure-cli-core/azure/cli/core/extension/__init__.py

yungezz · 2020-04-16T13:10:22Z

src/azure-cli-core/azure/cli/core/extension/__init__.py

                                                                                          'cliextensions')
 DEV_EXTENSION_SOURCES = _DEV_EXTENSION_SOURCES.split(',') if _DEV_EXTENSION_SOURCES else []
-EXTENSIONS_SYS_DIR = os.path.join(get_python_lib(), 'azure-cli-extensions') if sys.platform.startswith('linux') else ""
+EXTENSIONS_SYS_DIR = os.path.expanduser(_CUSTOM_EXT_SYS_DIR) if _CUSTOM_EXT_SYS_DIR else os.path.join(get_python_lib(), 'azure-cli-extensions')


run as admin only required when user set customized install dir right? if yes, then less concern since no change to existing behavoir

src/azure-cli-core/azure/cli/core/extension/__init__.py

yungezz · 2020-04-16T13:12:20Z

src/azure-cli-core/azure/cli/core/extension/operations.py

    except CalledProcessError as e:
        logger.debug(e.output)
        logger.debug(e)
+        if "PermissionError: [WinError 5]" in e.output:


what's default install path after the change? suppose no change

yes. no change for the default extension path.

yungezz · 2020-04-16T13:13:58Z

src/azure-cli-core/azure/cli/core/util.py

    _print("Extensions directory '{}'".format(EXTENSIONS_DIR))
+    import os
+    if os.path.isdir(EXTENSIONS_SYS_DIR) and os.listdir(EXTENSIONS_SYS_DIR):
+        _print("Extensions system directory '{}'".format(EXTENSIONS_SYS_DIR))


it's little confusing, how about direct show as Extensions directory

The idea is to have no impact on users who don't use --system to install extensions. This will only be shown when users have installed at least one extension with --system.

haroldrandom · 2020-04-29T07:08:56Z

Would it be better install to inside site-package/ directly?

fengzhou-msft · 2020-04-29T07:18:21Z

Would it be better install to inside site-package/ directly?

I think it's more organized with the azure-cli-extensions folder. What's the benefit of installing inside site-package/ directly?

haroldrandom · 2020-05-01T01:13:03Z

What's the benefit of installing inside site-package/ directly?

a dedicated folder will do as this PR #12367.
This feature wild make #12367 also work on Windows.

BTW, we should consider the way how user are developing extension. #13286

fengzhou-msft added 3 commits March 28, 2020 23:52

support extension install in system directory

c8508f0

fix style

f27ecbe

revert system directory print

19e1fdc

fengzhou-msft requested review from Juliehzl, arrownj, haroldrandom, jiasli, qianwens and zikalino as code owners April 2, 2020 15:04

fengzhou-msft commented Apr 2, 2020

View reviewed changes

fengzhou-msft added 2 commits April 3, 2020 00:08

fix test

a40c461

fix test

5c6e0c3

fengzhou-msft force-pushed the install_system_extension branch from a40c461 to 5c6e0c3 Compare April 3, 2020 02:52

yonzhan assigned fengzhou-msft Apr 11, 2020

yonzhan added this to the S168 milestone Apr 11, 2020

fengzhou-msft added 3 commits April 12, 2020 22:39

Merge branch 'install_system_extension' of github.com:fengzhou-msft/a…

e67e69f

…zure-cli into install_system_extension

add test

4abb227

add windows instruction

ddc4750

fengzhou-msft changed the title ~~[WIP][Extension] Support installing extensions in a system directory~~ [Extension] Support installing extensions in a system directory Apr 14, 2020

arrownj reviewed Apr 15, 2020

View reviewed changes

fengzhou-msft commented Apr 16, 2020

View reviewed changes

src/azure-cli-core/azure/cli/core/extension/__init__.py Outdated Show resolved Hide resolved

Update src/azure-cli-core/azure/cli/core/extension/__init__.py

334236a

yungezz reviewed Apr 16, 2020

View reviewed changes

yonzhan modified the milestones: S168, S169 - For Build Apr 18, 2020

arrownj approved these changes Apr 29, 2020

View reviewed changes

Merge branch 'dev' into install_system_extension

beac785

haroldrandom approved these changes Apr 29, 2020

View reviewed changes

fengzhou-msft and others added 2 commits May 6, 2020 13:22

Merge branch 'dev' into install_system_extension

43957b1

modify code owner

78f9a55

zikalino approved these changes May 6, 2020

View reviewed changes

fengzhou-msft merged commit 7674236 into Azure:dev May 6, 2020

jiasli mentioned this pull request Nov 18, 2021

{Extension}: drop distutils.sysconfig, deprecated #20391

Closed

[Extension] Support installing extensions in a system directory #12856

[Extension] Support installing extensions in a system directory #12856

Uh oh!

Conversation

fengzhou-msft commented Apr 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yonzhan commented Apr 11, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haroldrandom commented Apr 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fengzhou-msft commented Apr 29, 2020

Uh oh!

haroldrandom commented May 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

fengzhou-msft commented Apr 2, 2020 •

edited

Loading

haroldrandom commented Apr 29, 2020 •

edited

Loading

haroldrandom commented May 1, 2020 •

edited

Loading