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.

Sign up for GitHub

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

Jump to bottom

[Packaging] Support Python 3.11 #26923

Merged
merged 31 commits into from
Oct 8, 2023
Merged

[Packaging] Support Python 3.11 #26923

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
edf18bd
Init
bebound Jul 18, 2023
e2852ec
Replace inspect.getargspec
bebound Jul 18, 2023
d0421af
Merge branch 'dev' into python-3.11
bebound Jul 20, 2023
c5dec5a
Fix mock_get_extension
bebound Jul 20, 2023
f91ee23
Fix mock error
bebound Jul 21, 2023
db4341f
Test new knack
bebound Jul 24, 2023
7a0c285
Fix test_parser_error_spellchecker
bebound Jul 24, 2023
265fb6c
Fix test_help_loads
bebound Jul 24, 2023
70a89dc
Fix test_command_index_always_loaded_extension
bebound Jul 25, 2023
f2dc5e7
Fix test_vm_defaults
bebound Jul 25, 2023
890df61
Revert "Test new knack"
bebound Jul 26, 2023
97beb14
Merge remote-tracking branch 'upstream/main' into python-3.11
bebound Aug 1, 2023
ae79bd7
Merge remote-tracking branch 'upstream/dev' into python-3.11
bebound Aug 3, 2023
9532240
Revert "Fix test_vm_defaults"
bebound Aug 3, 2023
5a3d9f2
Merge remote-tracking branch 'upstream/dev' into python-3.11
bebound Aug 3, 2023
957f19b
Use 3.11 in test
bebound Aug 4, 2023
0e92cb2
Fix enum str change in 3.11
bebound Aug 4, 2023
edea742
Merge remote-tracking branch 'upstream/dev' into python-3.11
bebound Aug 4, 2023
7e19a72
Minor fix
bebound Aug 7, 2023
36d8dc4
Use inspect.signature()
bebound Aug 7, 2023
87d3e90
Ignore deprecated-method
bebound Aug 7, 2023
952aac6
Merge remote-tracking branch 'upstream/dev' into python-3.11
bebound Aug 10, 2023
b5fedff
Merge branch 'dev' into python-3.11
bebound Aug 17, 2023
f4cf181
Merge remote-tracking branch 'upstream/dev' into python-3.11
bebound Aug 24, 2023
c8109ff
Rollback to inspect.getfullargspec
bebound Aug 25, 2023
ba4951a
Replace locale.getdefaultlocale with locale.getlocale
bebound Sep 12, 2023
f87722c
Merge commit 'a0b940589ec808e41b379b67286fe6d975ed621f' into python-3.11
bebound Sep 13, 2023
f2baaba
Use 3.10 in CodegenCoverage
bebound Sep 13, 2023
6936dc5
Revert fix mock_get_extension
bebound Sep 13, 2023
8b7a8c3
Use 3.11 in CodegenCoverage
bebound Sep 14, 2023
d0887b9
Merge remote-tracking branch 'upstream/dev' into python-3.11
bebound Sep 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .azure-pipelines/templates/azdev_setup.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ steps:
if [ "${{ parameters.EnableCompactAAZ }}" == 'True' ]; then
python $CLI_REPO_PATH/scripts/compact_aaz.py
fi
pip uninstall -y knack
pip install git+https://github.com/jiasli/knack@py311
displayName: 'azdev setup'
env:
CLI_REPO_PATH: ${{ parameters.CLIRepoPath }}
Expand Down
18 changes: 9 additions & 9 deletions azure-pipelines-full-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ jobs:
matrix:
Python39:
python.version: '3.9'
Python310:
python.version: '3.10'
Python311:
python.version: '3.11'
steps:
- template: .azure-pipelines/templates/automation_test.yml
parameters:
Expand All @@ -44,8 +44,8 @@ jobs:
matrix:
Python39:
python.version: '3.9'
Python310:
python.version: '3.10'
Python311:
python.version: '3.11'
steps:
- template: .azure-pipelines/templates/automation_test.yml
parameters:
Expand All @@ -63,8 +63,8 @@ jobs:
matrix:
Python39:
python.version: '3.9'
Python310:
python.version: '3.10'
Python311:
python.version: '3.11'
steps:
- template: .azure-pipelines/templates/automation_test.yml
parameters:
Expand Down Expand Up @@ -107,8 +107,8 @@ jobs:
fullTest: true
jobName: 'FullTest'

- job: AutomationFullTestPython310ProfileLatest
displayName: Automation Full Test Python310 Profile Latest
- job: AutomationFullTestPython311ProfileLatest
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should drop Python 3.10 tests right away as the bundled Python is still 3.10.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bundled Python is also bumped in #26749

displayName: Automation Full Test Python311 Profile Latest
timeoutInMinutes: 9999
strategy:
maxParallel: 8
Expand Down Expand Up @@ -147,7 +147,7 @@ jobs:
- AutomationTest20190301
- AutomationTest20180301
- AutomationFullTestPython39ProfileLatest
- AutomationFullTestPython310ProfileLatest
- AutomationFullTestPython311ProfileLatest
condition: and(failed(), in(variables['Build.Reason'], 'BatchedCI'))
displayName: Notify CI Errors
pool:
Expand Down
20 changes: 10 additions & 10 deletions azure-pipelines.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,8 +455,8 @@ jobs:
matrix:
Python39:
python.version: '3.9'
Python310:
python.version: '3.10'
Python311:
python.version: '3.11'
steps:
- template: .azure-pipelines/templates/automation_test.yml
parameters:
Expand All @@ -472,8 +472,8 @@ jobs:
matrix:
Python39:
python.version: '3.9'
Python310:
python.version: '3.10'
Python311:
python.version: '3.11'
steps:
- template: .azure-pipelines/templates/automation_test.yml
parameters:
Expand All @@ -492,8 +492,8 @@ jobs:
matrix:
Python39:
python.version: '3.9'
Python310:
python.version: '3.10'
Python311:
python.version: '3.11'
steps:
- task: UsePythonVersion@0
displayName: 'Use Python $(python.version)'
Expand All @@ -513,8 +513,8 @@ jobs:
name: ${{ variables.ubuntu_pool }}
strategy:
matrix:
Python310:
python.version: '3.10'
Python311:
python.version: '3.11'
steps:
- task: UsePythonVersion@0
displayName: 'Use Python $(python.version)'
Expand Down Expand Up @@ -1015,8 +1015,8 @@ jobs:
matrix:
Python39:
python.version: '3.9'
Python310:
python.version: '3.10'
Python311:
python.version: '3.11'
pool:
name: ${{ variables.ubuntu_pool }}
steps:
Expand Down
1 change: 1 addition & 0 deletions scripts/ci/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ CLASSIFIERS = [
'Programming Language :: Python :: 3.8',
'Programming Language :: Python :: 3.9',
'Programming Language :: Python :: 3.10',
'Programming Language :: Python :: 3.11',
'License :: OSI Approved :: MIT License',
]

Expand Down
12 changes: 6 additions & 6 deletions src/azure-cli-core/azure/cli/core/tests/test_command_registration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,10 @@ def _mock_get_extension_modname(ext_name, ext_dir):
return "azext_always_loaded"

def _mock_get_extensions():
MockExtension = namedtuple('Extension', ['name', 'preview', 'experimental', 'path', 'get_metadata'])
return [MockExtension(name=__name__ + '.ExtCommandsLoader', preview=False, experimental=False, path=None, get_metadata=lambda: {}),
MockExtension(name=__name__ + '.Ext2CommandsLoader', preview=False, experimental=False, path=None, get_metadata=lambda: {}),
MockExtension(name=__name__ + '.ExtAlwaysLoadedCommandsLoader', preview=False, experimental=False, path=None, get_metadata=lambda: {})]
MockExtension = namedtuple('Extension', ['name', 'preview', 'experimental', 'path', 'get_metadata', 'version', 'ext_type'])
return [MockExtension(name=__name__ + '.ExtCommandsLoader', preview=False, experimental=False, path=None, get_metadata=lambda: {}, version='0.0.1', ext_type='dev'),
MockExtension(name=__name__ + '.Ext2CommandsLoader', preview=False, experimental=False, path=None, get_metadata=lambda: {}, version='0.0.1', ext_type='dev'),
MockExtension(name=__name__ + '.ExtAlwaysLoadedCommandsLoader', preview=False, experimental=False, path=None, get_metadata=lambda: {}, version='0.0.1', ext_type='dev')]
Copy link
Contributor Author

@bebound bebound Jul 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The MockExtension does not mock all attribution in Extension.

Fix stderr in "Unit Test for Core 3.11" when run azdev test azure-cli-core

ERROR cli.azure.cli.core:init.py:273 Error loading command module 'serviceconnector': 'Extension' object has no attribute 'version'

Error stack is:

Traceback (most recent call last):
  File "c:\users\kk\developer\azure-cli\src\azure-cli-core\azure\cli\core\__init__.py", line 256, in _update_command_table_from_modules
    module_command_table, module_group_table = _load_module_command_loader(self, args, mod)
                                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "c:\users\kk\developer\azure-cli\src\azure-cli-core\azure\cli\core\commands\__init__.py", line 1085, in _load_module_command_loader
    return _load_command_loader(loader, args, mod, 'azure.cli.command_modules.')
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "c:\users\kk\developer\azure-cli\src\azure-cli-core\azure\cli\core\commands\__init__.py", line 1052, in _load_command_loader
    module = import_module(prefix + name)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Program Files\WindowsApps\PythonSoftwareFoundation.Python.3.11_3.11.1264.0_x64__qbz5n2kfra8p0\Lib\importlib\__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap>", line 1204, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1176, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1147, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 690, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 940, in exec_module
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "c:\users\kk\developer\azure-cli\src\azure-cli\azure\cli\command_modules\serviceconnector\__init__.py", line 7, in <module>
    from azure.cli.command_modules.serviceconnector._help import helps  # pylint: disable=unused-import
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "c:\users\kk\developer\azure-cli\src\azure-cli\azure\cli\command_modules\serviceconnector\_help.py", line 70, in <module>
    if not should_load_source(source):
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "c:\users\kk\developer\azure-cli\src\azure-cli\azure\cli\command_modules\serviceconnector\_utils.py", line 34, in should_load_source
    installed_extensions = [item.get('name') for item in list_extensions()]
                                                         ^^^^^^^^^^^^^^^^^
  File "c:\users\kk\developer\azure-cli\src\azure-cli-core\azure\cli\core\extension\operations.py", line 397, in list_extensions
    return [{OUT_KEY_NAME: ext.name, OUT_KEY_VERSION: ext.version, OUT_KEY_TYPE: ext.ext_type,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "c:\users\kk\developer\azure-cli\src\azure-cli-core\azure\cli\core\extension\operations.py", line 397, in <listcomp>
    return [{OUT_KEY_NAME: ext.name, OUT_KEY_VERSION: ext.version, OUT_KEY_TYPE: ext.ext_type,
                                                      ^^^^^^^^^^^
AttributeError: 'Extension' object has no attribute 'version'

Copy link
Member

Choose a reason for hiding this comment

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

Why didn't this fail before?

Copy link
Contributor Author

@bebound bebound Sep 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It is a Captured log call, it does not affect the test result.
The 3.11 test fails so I notice this error.

I've reverted this change.
I don't know cause of this error, I'll create a new PR if I can figure it out.


def _mock_load_command_loader(loader, args, name, prefix):

Expand Down Expand Up @@ -298,10 +298,10 @@ def _check_index():
loader.load_command_table(["hello", "mod-only"])
_check_index()

with mock.patch("azure.cli.core.__version__", "2.7.0"), mock.patch.object(cli.cloud, "profile", "2019-03-01-hybrid"):
Copy link
Member

Choose a reason for hiding this comment

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

This test fails in Python 3.11:

E           AttributeError: <MagicMock id='2148541021584'> does not have the attribute '__version__'

C:\Users\xxx\AppData\Local\Programs\Python\Python311\Lib\unittest\mock.py:1416: AttributeError

I found a similar issue scikit-hep/pyhf#2143. Switching to mock.patch.object(azure.cli.core, '__version__', "2.7.0") indeed solves this error. This makes me wondering if the import logic with string 'azure.cli.core' has been broken in Python 3.11.

After further debugging mock.py source code, I noticed unittest.mock._patch.getter in Python 3.10 is able to get azure.cli.core module, but unittest.mock._patch.getter in Python 3.11 is not able to do that - it returns a MagicMock. According to https://docs.python.org/3/library/unittest.mock.html#id3:

The only exceptions are magic methods and attributes (those that have leading and trailing double underscores). Mock doesn’t create these but instead raises an AttributeError. This is because the interpreter will often implicitly request these methods, and gets very confused to get a new Mock object when it expects a magic method. If you need magic method support see magic methods.

Git blaming mock.py shows unittest.mock._get_target's logic has been changed to use pkgutil.resolve_name in Python 3.11 (python/cpython#18544), but pkgutil.resolve_name's functionality is broken by

azure-cli/src/azure-cli-core/azure/cli/core/tests/test_command_registration.py

Line 264 in 8700432

@mock.patch('pkgutil.iter_modules', _mock_iter_modules)

Copy link
Contributor Author

@bebound bebound Jul 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Great work, I have never considered that this error is related to @mock.patch('pkgutil.iter_modules', _mock_iter_modules) .

Copy link
Contributor Author

@bebound bebound Aug 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I figured this out.

The actual cause is @mock.patch('importlib.import_module', _mock_import_lib).

In 3.10, mock use its own _import(target_name), which uses __import__ and getattr, to get the target object. azure.cli.core is returned.
https://github.com/python/cpython/blob/fc382d3dd08709a9dc5000a691d3a74f7b4a99ac/Lib/unittest/mock.py#L1618

In 3.11, it uses pkgutil.resolve_name(target_name) to get target object, which calls importlib.import_module internally.
However, importlib.import_module is patched and always returns mock.MagicMock(), thus a MagicMock() is returned and it does not have __version__ attribution.
https://github.com/python/cpython/blob/ea77520094085ff86160f64fd4bc4f7e8e4ec0d2/Lib/unittest/mock.py#L1614

Copy link
Member

Choose a reason for hiding this comment

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

Let's keep this conversation open as a TODO item. I feel there should be a way to patch pkgutil.resolve_name more elegantly.

with mock.patch.object(cli.cloud, "profile", "2019-03-01-hybrid"):
def update_and_check_index():
loader.load_command_table(["hello", "mod-only"])
self.assertEqual(INDEX[CommandIndex._COMMAND_INDEX_VERSION], "2.7.0")
self.assertEqual(INDEX[CommandIndex._COMMAND_INDEX_VERSION], __version__)
self.assertEqual(INDEX[CommandIndex._COMMAND_INDEX_CLOUD_PROFILE], "2019-03-01-hybrid")
self.assertDictEqual(INDEX[CommandIndex._COMMAND_INDEX], self.expected_command_index)

Expand Down
6 changes: 3 additions & 3 deletions src/azure-cli-core/azure/cli/core/tests/test_parser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,9 @@ def _mock_extension_modname(ext_name, ext_dir):
return ext_name

def _mock_get_extensions(**kwargs):
MockExtension = namedtuple('Extension', ['name', 'preview', 'experimental', 'path', 'get_metadata'])
return [MockExtension(name=__name__ + '.ExtCommandsLoader', preview=False, experimental=False, path=None, get_metadata=lambda: {}),
MockExtension(name=__name__ + '.Ext2CommandsLoader', preview=False, experimental=False, path=None, get_metadata=lambda: {})]
MockExtension = namedtuple('Extension', ['name', 'preview', 'experimental', 'path', 'get_metadata', 'version', 'ext_type'])
return [MockExtension(name=__name__ + '.ExtCommandsLoader', preview=False, experimental=False, path=None, get_metadata=lambda: {}, version='0.0.1', ext_type='dev'),
MockExtension(name=__name__ + '.Ext2CommandsLoader', preview=False, experimental=False, path=None, get_metadata=lambda: {}, version='0.0.1', ext_type='dev')]

def _mock_load_command_loader(loader, args, name, prefix):
from enum import Enum
Expand Down
2 changes: 1 addition & 1 deletion src/azure-cli-core/azure/cli/core/util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,7 @@ def get_arg_list(op):
sig = inspect.signature(op)
return sig.parameters
except AttributeError:
sig = inspect.getargspec(op) # pylint: disable=deprecated-method
sig = inspect.getfullargspec(op)
return sig.args


Expand Down
1 change: 1 addition & 0 deletions src/azure-cli-core/setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
'Programming Language :: Python :: 3.8',
'Programming Language :: Python :: 3.9',
'Programming Language :: Python :: 3.10',
'Programming Language :: Python :: 3.11',
'License :: OSI Approved :: MIT License',
]

Expand Down
1 change: 1 addition & 0 deletions src/azure-cli-telemetry/setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
'Programming Language :: Python :: 3.8',
'Programming Language :: Python :: 3.9',
'Programming Language :: Python :: 3.10',
'Programming Language :: Python :: 3.11',
'License :: OSI Approved :: MIT License',
]

Expand Down
2 changes: 1 addition & 1 deletion src/azure-cli-testsdk/azure/cli/testsdk/scenario_tests/utilities.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def trim_kwargs_from_test_function(fn, kwargs):
# the next function is the actual test function. the kwargs need to be trimmed so
# that parameters which are not required will not be passed to it.
if not is_preparer_func(fn):
args, _, kw, _ = inspect.getargspec(fn) # pylint: disable=deprecated-method
args, _, kw, *_ = inspect.getfullargspec(fn)
Copy link
Contributor Author

@bebound bebound Jul 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

FAILED src\azure-cli\azure\cli\command_modules\vm\tests\latest\test_vm_commands.py::VMSSCreateBalancerOptionsTest::test_vmss_create_default_app_gateway - AttributeError: module 'inspect' has no attribute 'getargspec'

inspect.getargspec is deprecated in 3.11. Use getfullargspec instead.
It returns FullArgSpec(args, varargs, varkw, defaults, kwonlyargs, kwonlydefaults, annotations) instead of ArgSpec(args, varargs, keywords, defaults)

Ref:
https://docs.python.org/3.10/library/inspect.html#inspect.getfullargspec

Copy link
Member

@jiasli jiasli Jul 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It's time to drop inspect.getargspec and even inspect.getfullargspec. See microsoft/knack#275 (comment).

A previous attempt was made by #24111.

if kw is None:
args = set(args)
for key in [k for k in kwargs if k not in args]:
Expand Down
1 change: 1 addition & 0 deletions src/azure-cli-testsdk/setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
'Programming Language :: Python :: 3.8',
'Programming Language :: Python :: 3.9',
'Programming Language :: Python :: 3.10',
'Programming Language :: Python :: 3.11',
'License :: OSI Approved :: MIT License',
]

Expand Down
2 changes: 1 addition & 1 deletion .../cli/command_modules/keyvault/vendored_sdks/azure_keyvault_t1/key_vault_authentication.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def __init__(self, authorization_callback):
# for backwards compatibility we need to support callbacks which don't accept the scheme
def _auth_callback_compat(self, server, resource, scope, scheme):
return self._user_callback(server, resource, scope) \
if len(inspect.getargspec(self._user_callback).args) == 3 \
if len(inspect.getfullargspec(self._user_callback).args) == 3 \
else self._user_callback(server, resource, scope, scheme)

def __call__(self, request):
Expand Down
1 change: 1 addition & 0 deletions src/azure-cli/setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
'Programming Language :: Python :: 3.8',
'Programming Language :: Python :: 3.9',
'Programming Language :: Python :: 3.10',
'Programming Language :: Python :: 3.11',
'License :: OSI Approved :: MIT License',
]

Expand Down
Loading