[Packaging] Support Python 3.11 #26923
Conversation
️✔️AzureCLI-FullTest
|
|
Hi @bebound, |
ghost
requested a review
from
️✔️AzureCLI-BreakingChangeTest
|
|
Packaging |
ghost
added
the
| @@ -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) | |||
There was a problem hiding this comment.
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
There was a problem hiding this comment.
It's time to drop inspect.getargspec and even inspect.getfullargspec. See microsoft/knack#275 (comment).
A previous attempt was made by #24111.
| 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')] |
There was a problem hiding this comment.
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'
There was a problem hiding this comment.
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.
| @@ -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"): | |||
| with mock.patch.object(cli.cloud, "profile", "2019-03-01-hybrid"): | |||
There was a problem hiding this comment.
Prevent AttributeError: <MagicMock id='2694374960976'> does not have the attribute '__version__' error.
Not sure why it breaks in 3.11.
It seems mock can only overwrite some of the magic functions in Python and __version__ is not supported, see
https://docs.python.org/3/library/unittest.mock.html#id3
https://docs.python.org/3/library/unittest.mock.html#magic-methods
| with mock.patch('logging.Logger.error', mock_log_error), \ | ||
| mock.patch('difflib.get_close_matches', mock_get_close_matches): | ||
| with mock.patch.object(logging.Logger, 'error', mock_log_error), \ | ||
| mock.patch.object(difflib, 'get_close_matches', mock_get_close_matches): |
There was a problem hiding this comment.
Fix this error which is also caused by https://github.com/Azure/azure-cli/pull/26923/files#r1271877325
self.assertEqual(len(logger_msgs), 5)
E AssertionError: 0 != 5
src\azure-cli-core\azure\cli\core\tests\test_parser.py:245: AssertionError
| @@ -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"): | |||
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Great work, I have never considered that this error is related to @mock.patch('pkgutil.iter_modules', _mock_iter_modules) .
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Let's keep this conversation open as a TODO item. I feel there should be a way to patch pkgutil.resolve_name more elegantly.
| @@ -130,7 +130,6 @@ def test_help_loads(self): | |||
| except SystemExit: | |||
| pass | |||
| cmd_tbl = cli.invocation.commands_loader.command_table | |||
| cli.invocation.parser.load_command_table(cli.invocation.commands_loader) | |||
There was a problem hiding this comment.
Fix
src\azure-cli-core\azure\cli\core\tests\test_help.py:143:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
src\azure-cli-core\azure\cli\core\parser.py:96: in load_command_table
command_parser = subparser.add_parser(command_verb,
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
self = _SubParsersAction(option_strings=[], dest='_subcommand', nargs='A...', const=None, default=None, type=None, choices={'...ss=<class 'argparse.HelpFormatter'>, conflict_handler='error', add_help=True)}, required=True, help=None, metavar=None)
name = 'check-name'
kwargs = {'_command_source': 'acr', 'cli_help': <azure.cli.core._help.AzCliHelp object at 0x00000124500EA150>, 'conflict_handle....description_loader of <azure.cli.core.commands.command_operation.CommandOperation object at 0x000001245073A150>>, ...}
aliases = ()
def add_parser(self, name, **kwargs):
# set prog from the existing prefix
if kwargs.get('prog') is None:
kwargs['prog'] = '%s %s' % (self._prog_prefix, name)
aliases = kwargs.pop('aliases', ())
if name in self._name_parser_map:
> raise ArgumentError(self, _('conflicting subparser: %s') % name)
E argparse.ArgumentError: argument _subcommand: conflicting subparser: check-name
The command table is build in cli.invoke(['-h']). If this line is called again, subparsers are added again, which is not allowed in Python 3.11. See python/cpython#18605
| @@ -58,8 +58,8 @@ 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 \ | |||
| args = [i for i in inspect.signature(self._user_callback).parameters if i not in {'args', 'kwargs'}] | |||
There was a problem hiding this comment.
*, ** arguments may not necessarily be called arg or kwargs. It is safer to check whether its kind is VAR_POSITIONAL or VAR_KEYWORD.
Maybe getfullargspec is an easier drop-in replacement.
There was a problem hiding this comment.
Is it possible to totally remove this signature check as our callback now only has one format?
There was a problem hiding this comment.
Rollback to inspect.getfullargspec, for using signature to achieve same result is obscure.
import inspect
def testfunc(a, /, b=1, c=2, *args, kk, **kwargs):
pass
print(inspect.getfullargspec(testfunc))
print(inspect.signature(testfunc).parameters)
for i, j in inspect.signature(testfunc).parameters.items():
print(i, type(i), j, type(j), j.kind)
args, _, kw, *_ = inspect.getfullargspec(testfunc)
print(args, kw)
from inspect import Parameter
parameters = inspect.signature(testfunc).parameters
args = [k for k, v in parameters.items() if v.kind in {Parameter.POSITIONAL_OR_KEYWORD, Parameter.POSITIONAL_ONLY}]
kw = next(iter([k for k, v in parameters.items() if v.kind == Parameter.VAR_KEYWORD]), None)
print(args, kw)FullArgSpec(args=['a', 'b', 'c'], varargs='args', varkw='kwargs', defaults=(1, 2), kwonlyargs=['kk'], kwonlydefaults=None, annotations={})
OrderedDict([('a', <Parameter "a">), ('b', <Parameter "b=1">), ('c', <Parameter "c=2">), ('args', <Parameter "*args">), ('kk', <Parameter "kk">), ('kwargs', <Parameter "**kwargs">)])
a <class 'str'> a <class 'inspect.Parameter'> POSITIONAL_ONLY
b <class 'str'> b=1 <class 'inspect.Parameter'> POSITIONAL_OR_KEYWORD
c <class 'str'> c=2 <class 'inspect.Parameter'> POSITIONAL_OR_KEYWORD
args <class 'str'> *args <class 'inspect.Parameter'> VAR_POSITIONAL
kk <class 'str'> kk <class 'inspect.Parameter'> KEYWORD_ONLY
kwargs <class 'str'> **kwargs <class 'inspect.Parameter'> VAR_KEYWORD
['a', 'b', 'c'] kwargs
['a', 'b', 'c'] kwargs
bebound
force-pushed
the
python-3.11
branch
from
269b7f3 to
f87722c
Compare
| @@ -107,8 +107,8 @@ jobs: | |||
| fullTest: true | |||
| jobName: 'FullTest' | |||
|
|
|||
| - job: AutomationFullTestPython310ProfileLatest | |||
| displayName: Automation Full Test Python310 Profile Latest | |||
| - job: AutomationFullTestPython311ProfileLatest | |||
There was a problem hiding this comment.
I don't think we should drop Python 3.10 tests right away as the bundled Python is still 3.10.
There was a problem hiding this comment.
Bundled Python is also bumped in #26749
bebound
modified the milestones:
September 2023 (2023-09-26),
October 2023 (2023-11-15)❗ Breaking change release ❗
|
We forgot to bump Python version in azure-cli/scripts/regression_test/regression_test.yml Lines 21 to 23 in e773a71 |
Description
This PR bumps related test to use 3.11, and add 3.11 support in
setup.pyClose #24494
Changes in 3.11:
inspect.getargspecmock.patchnot working ifpkgutil.resolve_nameis mockedargparse.ArgumentError: argument _subcommand: conflicting subparser: check-name__format__changesRef:
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.
I adhere to the Error Handling Guidelines.