openstack
/
cinder
Code Issues Proposed changes

drop use of pkg_resources 

Importing pkg_resources scans all of the installed modules for data
that won't be used. Switch to using importlib.metdata, which more
efficiently loads the metadata for a package.

Since the name of the module where importlib.metadata is found depends
on the version of python, mocking a function in the library is more
complicated. Provide a wrapper in the module that uses
importlib.metadata.version() so its tests can examine behavior using
different versions via mocks.

The distutils package in the standard library is deprecated. Use the
packaging library for parsing version strings into something that can
be compared.

Change-Id: I45d0851cdb5f241ff8dc774dc22123b410502cd9
Signed-off-by: Doug Hellmann <doug@doughellmann.com>
This commit is contained in:
Doug Hellmann 2020-07-25 13:23:23 -04:00
parent 2f600816a1
commit 521057dd97
No known key found for this signature in database
GPG Key ID: 3B6D06A0C428437A
5 changed files with 41 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
cinder/backup/drivers/gcs.py
View File

@ -27,10 +27,16 @@ Server-centric flow is used for authentication.
"""
import base64
from distutils import version
import hashlib
import os
try:
# For python 3.8 and later
import importlib.metadata as importlib_metadata
except ImportError:
# For everyone else
import importlib_metadata
try:
from google.auth import exceptions as gexceptions
from google.oauth2 import service_account
@ -49,7 +55,7 @@ from googleapiclient import http
from oslo_config import cfg
from oslo_log import log as logging
from oslo_utils import timeutils
import pkg_resources
from packaging import version
import six
from cinder.backup import chunkeddriver
@ -142,6 +148,17 @@ def gcs_logger(func):
return func_wrapper
def _get_dist_version(name):
"""Mock-able wrapper for importlib_metadata.version()
The module name where version() is found varies by python
version. This function makes it easier for tests to mock the
function and change the return value.
"""
return importlib_metadata.version(name)
@interface.backupdriver
class GoogleBackupDriver(chunkeddriver.ChunkedBackupDriver):
"""Provides backup, restore and delete of backup objects within GCS."""
@ -174,8 +191,8 @@ class GoogleBackupDriver(chunkeddriver.ChunkedBackupDriver):
# If we have google client that support google-auth library
# (v1.6.0 or higher) and all required libraries are installed use
# google-auth for the credentials
dist = pkg_resources.get_distribution('google-api-python-client')
if (version.LooseVersion(dist.version) >= version.LooseVersion('1.6.0')
dist_version = _get_dist_version('google-api-python-client')
if (version.parse(dist_version) >= version.parse('1.6.0')
and service_account):
creds = service_account.Credentials.from_service_account_file(
backup_credential)

24
cinder/tests/unit/backup/drivers/test_backup_google.py
View File

@ -623,59 +623,59 @@ class GoogleBackupDriverTestCase(test.TestCase):
self.assertEqual('none', result[0])
self.assertEqual(already_compressed_data, result[1])
@mock.patch('pkg_resources.get_distribution')
@mock.patch.object(google_dr, '_get_dist_version')
@mock.patch.object(google_dr.client.GoogleCredentials, 'from_stream')
@mock.patch.object(google_dr.discovery, 'build')
@mock.patch.object(google_dr, 'service_account')
def test_non_google_auth_version(self, account, build, from_stream,
get_dist_mock):
get_dist_version):
# Prior to v1.6.0 Google api client doesn't support google-auth library
get_dist_mock.return_value.version = '1.5.5'
get_dist_version.return_value = '1.5.5'
google_dr.CONF.set_override('backup_gcs_credential_file',
'credentials_file')
google_dr.GoogleBackupDriver(self.ctxt)
get_dist_mock.assert_called_once_with('google-api-python-client')
get_dist_version.assert_called_once_with('google-api-python-client')
from_stream.assert_called_once_with('credentials_file')
account.Credentials.from_service_account_file.assert_not_called()
build.assert_called_once_with('storage', 'v1', cache_discovery=False,
credentials=from_stream.return_value)
@mock.patch('pkg_resources.get_distribution')
@mock.patch.object(google_dr, '_get_dist_version')
@mock.patch.object(google_dr.client.GoogleCredentials, 'from_stream')
@mock.patch.object(google_dr.discovery, 'build')
@mock.patch.object(google_dr, 'service_account', None)
def test_no_httplib2_auth(self, build, from_stream, get_dist_mock):
def test_no_httplib2_auth(self, build, from_stream, get_dist_version):
# Google api client requires google-auth-httplib2 if not present we
# use legacy credentials
get_dist_mock.return_value.version = '1.6.6'
get_dist_version.return_value = '1.6.6'
google_dr.CONF.set_override('backup_gcs_credential_file',
'credentials_file')
google_dr.GoogleBackupDriver(self.ctxt)
get_dist_mock.assert_called_once_with('google-api-python-client')
get_dist_version.assert_called_once_with('google-api-python-client')
from_stream.assert_called_once_with('credentials_file')
build.assert_called_once_with('storage', 'v1', cache_discovery=False,
credentials=from_stream.return_value)
@mock.patch('pkg_resources.get_distribution')
@mock.patch.object(google_dr, '_get_dist_version')
@mock.patch.object(google_dr, 'gexceptions', mock.Mock())
@mock.patch.object(google_dr.client.GoogleCredentials, 'from_stream')
@mock.patch.object(google_dr.discovery, 'build')
@mock.patch.object(google_dr, 'service_account')
def test_google_auth_used(self, account, build, from_stream,
get_dist_mock):
get_dist_version):
# Google api client requires google-auth-httplib2 if not present we
# use legacy credentials
get_dist_mock.return_value.version = '1.6.6'
get_dist_version.return_value = '1.6.6'
google_dr.CONF.set_override('backup_gcs_credential_file',
'credentials_file')
google_dr.GoogleBackupDriver(self.ctxt)
get_dist_mock.assert_called_once_with('google-api-python-client')
get_dist_version.assert_called_once_with('google-api-python-client')
from_stream.assert_not_called()
create_creds = account.Credentials.from_service_account_file
create_creds.assert_called_once_with('credentials_file')

6
cinder/volume/drivers/dell_emc/powermax/utils.py
View File

@ -21,6 +21,7 @@ import re
from oslo_log import log as logging
from oslo_utils import strutils
from oslo_utils import units
import packaging.version
import six
from cinder import exception
@ -1933,8 +1934,9 @@ class PowerMaxUtils(object):
:param minimum_version: minimum version allowed
:returns: boolean
"""
from pkg_resources import parse_version
return parse_version(version) >= parse_version(minimum_version)
checking = packaging.version.parse(version)
minimum = packaging.version.parse(minimum_version)
return checking >= minimum
@staticmethod
def parse_specs_from_pool_name(pool_name):

2
lower-constraints.txt
View File

@ -42,6 +42,7 @@ hacking==3.0.1
httplib2==0.9.1
idna==2.6
imagesize==1.0.0
importlib-metadata==1.7.0
iso8601==0.1.12
Jinja2==2.10
jsonpatch==1.21
@ -85,6 +86,7 @@ oslo.versionedobjects==1.31.2
oslo.vmware==2.35.0
oslotest==3.2.0
osprofiler==1.4.0
packaging==20.4
paramiko==2.4.0
Paste==2.0.2
PasteDeploy==1.5.0

2
requirements.txt
View File

@ -7,6 +7,7 @@ decorator>=3.4.0 # BSD
eventlet!=0.23.0,!=0.25.0,>=0.22.0 # MIT
greenlet>=0.4.13 # MIT
httplib2>=0.9.1 # MIT
importlib_metadata>=1.7.0;python_version<'3.8' # Apache-2.0
iso8601>=0.1.12 # MIT
jsonschema>=3.2.0 # MIT
keystoneauth1>=3.14.0 # Apache-2.0
@ -30,6 +31,7 @@ oslo.upgradecheck>=0.1.0 # Apache-2.0
oslo.utils>=3.34.0 # Apache-2.0
oslo.versionedobjects>=1.31.2 # Apache-2.0
osprofiler>=1.4.0 # Apache-2.0
packaging>=20.4
paramiko>=2.4.0 # LGPLv2.1+
Paste>=2.0.2 # MIT
PasteDeploy>=1.5.0 # MIT