Skip to content

Conversation

@jiasli
Copy link
Member

@jiasli jiasli commented Sep 14, 2021

Description

six was previously dropped from azure-cli-core (#17366). It should be dropped from azure-cli as well.

However, as there are countless extensions still using six, we still need to install it. This PR removes all usages of `six.

'PyJWT>=2.1.0',
'pyopenssl>=17.1.0', # https://github.com/pyca/pyopenssl/pull/612
'requests[socks]~=2.25.1',
'six~=1.12',
Copy link
Member Author

Choose a reason for hiding this comment

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

Remove the dependency from azure-cli-core.

'pytz==2019.1',
'scp~=0.13.2',
'semver==2.13.0',
'six>=1.10.0', # six is still used by countless extensions
Copy link
Member Author

Choose a reason for hiding this comment

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

Add six to azure-cli as it can be used by extensions.

@yonzhan
Copy link
Collaborator

yonzhan commented Sep 14, 2021

Misc

@yonzhan yonzhan added this to the Sep 2021 (2021-10-12) milestone Sep 14, 2021
Comment on lines 20 to +21
# the urlopen is imported for automation purpose
from six.moves.urllib.request import urlopen # noqa, pylint: disable=import-error,unused-import,ungrouped-imports
from urllib.request import urlopen # noqa, pylint: disable=import-error,unused-import,ungrouped-imports
Copy link
Member Author

@jiasli jiasli Sep 14, 2021

Choose a reason for hiding this comment

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

urlopen is never used by custom.py but is patched by test

@mock.patch('azure.cli.command_modules.vm.custom.urlopen', autospec=True)

What is exactly "automation purpose"?

@zhoxing-ms, Could you help confirm if we can remove this line?

Copy link
Contributor

@zhoxing-ms zhoxing-ms Sep 14, 2021

Choose a reason for hiding this comment

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

This is because in the previous PR #5328, the reference of urlopen in the method load_images_from_aliases_doc() was modified to use requests, but the mock path in test_read_images_from_alias_doc() was not synchronously modified.
Screenshot 2021-09-14 172536
code link

Therefore, the import of from six.moves.urllib.request import urlopen can be deleted, and we just need to change the mock path to @mock.patch(azure.cli.command_modules.vm.custom.requests, autospec=True)

Copy link
Member Author

Choose a reason for hiding this comment

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

We can do this in a separate PR if time permits.

# Conflicts:
#	src/azure-cli/setup.py
@jiasli jiasli merged commit ba44e95 into Azure:dev Oct 11, 2021
@jiasli jiasli deleted the drop-six branch October 11, 2021 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants