Skip to content
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} Stop importing pkg_resources when getting versions #14905

Merged
merged 4 commits into from
Aug 27, 2020

Conversation

jiasli
Copy link
Member

@jiasli jiasli commented Aug 25, 2020

Description

az version and the version check introduced in #14803 query the local versions of Azure CLI modules:

  • azure-cli
  • azure-cli-command-modules-nspkg
  • azure-cli-core
  • azure-cli-nspkg
  • azure-cli-telemetry

The current implementation uses pkg_resources.working_set which takes ~200ms to load:

python -X importtime -m azure.cli version 2>perf.log ; tuna .\perf.log

image

This is outstanding compared to az version's 500ms running time.

This PR uses the hard-coded version instead of querying all modules under site-packages using pkg_resources.

Removal of azure-cli-dev-tools and azure-cli-testsdk

This PR removes azure-cli-dev-tools and azure-cli-testsdk. The removal only applies to dev CLI, and won't affect the released CLI (they are not present at all). It is safe for public users.

After the change, the dev CLI and released CLI have the same output.

# Before (dev)
> az version
{
  "azure-cli": "2.11.0",
  "azure-cli-core": "2.11.0",
  "azure-cli-dev-tools": "0.1.1",
  "azure-cli-telemetry": "1.0.5",
  "azure-cli-testsdk": "0.2.4",
  "extensions": {
    "interactive": "0.4.4",
    "log-analytics": "0.2.1",
    "portal": "0.1.1",
    "resource-graph": "1.1.0"
  }
}

# After (dev and release)
> az version
{
  "azure-cli": "2.11.0",
  "azure-cli-core": "2.11.0",
  "azure-cli-telemetry": "1.0.5",
  "extensions": {
    "interactive": "0.4.4",
    "log-analytics": "0.2.1",
    "portal": "0.1.1",
    "resource-graph": "1.1.0"
  }
}

Performance

The performance improvement is significant - from ~500ms to ~300ms:

> Measure-Command {az version}
TotalMilliseconds : 531.9338

> Measure-Command {az version}
TotalMilliseconds : 358.5301

site-packages is not imported anymore:

image

Testing Guide

az version
Measure-Command {az version}

@jiasli
Copy link
Member Author

jiasli commented Aug 25, 2020

Side-notes

In PyCharm, setting a breakpoint at <virtual_env>\Lib\site-packages\pkg_resources\__init__.py and start debugging won't hit the breakpoint, because PyCharm imports pkg_resources before calling src/azure-cli/azure/cli/__main__.py. You may verify this by adding

print(sys.modules['pkg_resources'])

to src/azure-cli/azure/cli/__main__.py. You will see pkg_resources is already imported.

However, when directly executing

python -m azure.cli version

The print line results in a KeyError, because pkg_resources has not been imported yet.

Instead, you may put

import traceback
traceback.print_stack()

in <virtual_env>\Lib\site-packages\pkg_resources\__init__.py to check how pkg_resources is imported.

Full callstack for `python -m azure.cli version`
> python -m azure.cli version
  File "C:\Users\user1\AppData\Local\Programs\Python\Python38\lib\runpy.py", line 194, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "C:\Users\user1\AppData\Local\Programs\Python\Python38\lib\runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "d:\cli\azure-cli\src\azure-cli\azure\cli\__main__.py", line 48, in <module>
    exit_code = cli_main(az_cli, sys.argv[1:])
  File "d:\cli\azure-cli\src\azure-cli\azure\cli\__main__.py", line 35, in cli_main
    return cli.invoke(args)
  File "D:\cli\env38\lib\site-packages\knack\cli.py", line 215, in invoke
    cmd_result = self.invocation.execute(args)
  File "d:\cli\azure-cli\src\azure-cli-core\azure\cli\core\commands\__init__.py", line 647, in execute
    results, exceptions = self._run_jobs_serially(jobs, ids)
  File "d:\cli\azure-cli\src\azure-cli-core\azure\cli\core\commands\__init__.py", line 718, in _run_jobs_serially
    results.append(self._run_job(expanded_arg, cmd_copy))
  File "d:\cli\azure-cli\src\azure-cli-core\azure\cli\core\commands\__init__.py", line 688, in _run_job
    result = cmd_copy(params)
  File "d:\cli\azure-cli\src\azure-cli-core\azure\cli\core\commands\__init__.py", line 325, in __call__
    return self.handler(*args, **kwargs)
  File "d:\cli\azure-cli\src\azure-cli-core\azure\cli\core\__init__.py", line 782, in default_command_handler
    return op(**command_args)
  File "d:\cli\azure-cli\src\azure-cli\azure\cli\command_modules\util\custom.py", line 30, in show_version
    versions = get_az_version_json()
  File "d:\cli\azure-cli\src\azure-cli-core\azure\cli\core\util.py", line 300, in get_az_version_json
    for dist in get_installed_cli_distributions():
  File "d:\cli\azure-cli\src\azure-cli-core\azure\cli\core\util.py", line 131, in get_installed_cli_distributions
    from pkg_resources import working_set
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 783, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "D:\cli\env38\lib\site-packages\pkg_resources\__init__.py", line 22, in <module>
    traceback.print_stack()

Then if you start debugging in PyCharm, the output shows how PyCharm imports pkg_resources:

Full callstack for debugging in PyCharm
D:\cli\env38\Scripts\python.exe "C:\Program Files\JetBrains\PyCharm Community Edition 2019.2.5\helpers\pydev\pydevd.py" --multiproc --qt-support=auto --client 127.0.0.1 --port 14839 --file D:/cli/azure-cli/src/azure-cli/azure/cli/__main__.py version
  File "C:\Program Files\JetBrains\PyCharm Community Edition 2019.2.5\helpers\pydev\pydevd.py", line 34, in <module>
    from _pydevd_bundle import pydevd_vars
  File "<frozen importlib._bootstrap>", line 1042, in _handle_fromlist
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 783, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "C:\Program Files\JetBrains\PyCharm Community Edition 2019.2.5\helpers\pydev\_pydevd_bundle\pydevd_vars.py", line 11, in <module>
    from _pydevd_bundle.pydevd_xml import ExceptionOnEvaluate, get_type, var_to_xml
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 783, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "C:\Program Files\JetBrains\PyCharm Community Edition 2019.2.5\helpers\pydev\_pydevd_bundle\pydevd_xml.py", line 9, in <module>
    from _pydevd_bundle import pydevd_extension_utils
  File "<frozen importlib._bootstrap>", line 1042, in _handle_fromlist
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 783, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "C:\Program Files\JetBrains\PyCharm Community Edition 2019.2.5\helpers\pydev\_pydevd_bundle\pydevd_extension_utils.py", line 5, in <module>
    import pydevd_plugins.extensions as extensions
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 961, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 783, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "C:\Program Files\JetBrains\PyCharm Community Edition 2019.2.5\helpers\pydev\pydevd_plugins\__init__.py", line 2, in <module>
    __import__('pkg_resources').declare_namespace(__name__)
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 783, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "D:\cli\env38\lib\site-packages\pkg_resources\__init__.py", line 22, in <module>
    traceback.print_stack()
pydev debugger: process 26612 is connecting

@yonzhan yonzhan added this to the S175 - For Ignite milestone Aug 25, 2020
@yonzhan
Copy link
Collaborator

yonzhan commented Aug 25, 2020

Core

# return [d for d in list(working_set) if d.key == CLI_PACKAGE_NAME or d.key.startswith(COMPONENT_PREFIX)]

# Use the hard-coded version instead of querying all modules under site-packages.
from azure.cli.__main__ import __version__ as azure_cli_version
Copy link
Contributor

Choose a reason for hiding this comment

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

From azure-cli-core perspective, we should not assume there is a azure-cli repo. That is to say, azure.cli.__main__ may not exist.

Copy link
Member Author

@jiasli jiasli Aug 26, 2020

Choose a reason for hiding this comment

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

Good catch! As now azure-cli and azure-cli-core have the same version, we don't need this dependency anymore.

Also importing azure.cli.__main__ will cause __main__ to be executed, thus causing CI failure:

https://dev.azure.com/azure-sdk/public/_build/results?buildId=508652&view=logs&jobId=4ddf4a4c-924b-5e90-5386-b844cc2d2073&j=4ddf4a4c-924b-5e90-5386-b844cc2d2073&t=d1586226-7623-5d9f-0d54-f083e3aeff70

=====================
| Discovering Tests |
=====================

az: 'test' is not in the 'az' command group. See 'az --help'. If the command is from an extension, please make sure the corresponding extension is installed. To learn more about extensions, please visit https://docs.microsoft.com/en-us/cli/azure/azure-cli-extensions-overview

The most similar choice to 'test' is:
	rest

@jiasli jiasli requested a review from zhoxing-ms as a code owner August 26, 2020 06:21
Copy link
Contributor

@arrownj arrownj left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@fengzhou-msft fengzhou-msft left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@zhoxing-ms zhoxing-ms left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants