openstack/openstacksdk
Code Issues Proposed changes

Deprecate VolumeDetail and BackupDetail classes

Browse Source 
We have introduced altering base_path for operations on resources. 
Volumes and Backups were not switched to this new method and were still 
relying on returning inherited classes VolumeDetail and BackupDetail. 
This change allow doing any supported operation on a resource returned 
by list immediately without any conversion or re-fetch.

Change-Id: Ia095c53f1d04f1c119c6a6f8a38c5bfd60dc8a67
This commit is contained in:
Artem Goncharov 2019-03-09 11:12:38 +01:00
parent 1066c9385f
commit 0eaf2c95d3
15 changed files with 111 additions and 273 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
doc/source/user/resources/block_storage/v2/backup.rst
View File

@ -10,12 +10,3 @@ The ``Backup`` class inherits from :class:`~openstack.resource.Resource`.
.. autoclass:: openstack.block_storage.v2.backup.Backup
:members:
The BackupDetail Class
----------------------
The ``BackupDetail`` class inherits from
:class:`~openstack.block_storage.v2.backup.Backup`.
.. autoclass:: openstack.block_storage.v2.backup.BackupDetail
:members:

9
doc/source/user/resources/block_storage/v2/volume.rst
View File

@ -10,12 +10,3 @@ The ``Volume`` class inherits from :class:`~openstack.resource.Resource`.
.. autoclass:: openstack.block_storage.v2.volume.Volume
:members:
The VolumeDetail Class
----------------------
The ``VolumeDetail`` class inherits from
:class:`~openstack.block_storage.v2.volume.Volume`.
.. autoclass:: openstack.block_storage.v2.volume.VolumeDetail
:members:

24
openstack/block_storage/v2/_proxy.py
View File

@ -147,11 +147,9 @@ class Proxy(_base_proxy.BaseBlockStorageProxy):
def volumes(self, details=True, **query):
"""Retrieve a generator of volumes
:param bool details: When set to ``False``
:class:`~openstack.block_storage.v2.volume.Volume` objects
will be returned. The default, ``True``, will cause
:class:`~openstack.block_storage.v2.volume.VolumeDetail`
objects to be returned.
:param bool details: When set to ``False`` no extended attributes
will be returned. The default, ``True``, will cause objects with
additional attributes to be returned.
:param kwargs query: Optional query parameters to be sent to limit
the volumes being returned. Available parameters include:
@ -162,8 +160,8 @@ class Proxy(_base_proxy.BaseBlockStorageProxy):
:returns: A generator of volume objects.
"""
volume = _volume.VolumeDetail if details else _volume.Volume
return self._list(volume, **query)
base_path = '/volumes/detail' if details else None
return self._list(_volume.Volume, base_path=base_path, **query)
def create_volume(self, **attrs):
"""Create a new volume from attributes
@ -214,11 +212,9 @@ class Proxy(_base_proxy.BaseBlockStorageProxy):
def backups(self, details=True, **query):
"""Retrieve a generator of backups
:param bool details: When set to ``False``
:class:`~openstack.block_storage.v2.backup.Backup` objects
will be returned. The default, ``True``, will cause
:class:`~openstack.block_storage.v2.backup.BackupDetail`
objects to be returned.
:param bool details: When set to ``False`` no additional details will
be returned. The default, ``True``, will cause objects with
additional attributes to be returned.
:param dict query: Optional query parameters to be sent to limit the
resources being returned:
@ -239,8 +235,8 @@ class Proxy(_base_proxy.BaseBlockStorageProxy):
raise exceptions.SDKException(
'Object-store service is required for block-store backups'
)
backup = _backup.BackupDetail if details else _backup.Backup
return self._list(backup, **query)
base_path = '/backups/detail' if details else None
return self._list(_backup.Backup, base_path=base_path, **query)
def get_backup(self, backup):
"""Get a backup

9
openstack/block_storage/v2/backup.py
View File

@ -90,11 +90,4 @@ class Backup(resource.Resource):
return self
class BackupDetail(Backup):
"""Volume Backup with Details"""
base_path = "/backups/detail"
# capabilities
allow_list = True
#: Properties
BackupDetail = Backup

43
openstack/block_storage/v2/volume.py
View File

@ -75,32 +75,6 @@ class Volume(resource.Resource):
#: The timestamp of this volume creation.
created_at = resource.Body("created_at")
def _action(self, session, body):
"""Preform volume actions given the message body."""
# NOTE: This is using Volume.base_path instead of self.base_path
# as both Volume and VolumeDetail instances can be acted on, but
# the URL used is sans any additional /detail/ part.
url = utils.urljoin(Volume.base_path, self.id, 'action')
headers = {'Accept': ''}
return session.post(url, json=body, headers=headers)
def extend(self, session, size):
"""Extend a volume size."""
body = {'os-extend': {'new_size': size}}
self._action(session, body)
class VolumeDetail(Volume):
base_path = "/volumes/detail"
# capabilities
allow_fetch = False
allow_create = False
allow_delete = False
allow_commit = False
allow_list = True
#: The volume's current back-end.
host = resource.Body("os-vol-host-attr:host")
#: The project ID associated with current back-end.
@ -123,3 +97,20 @@ class VolumeDetail(Volume):
#: ``True`` if this volume is encrypted, ``False`` if not.
#: *Type: bool*
is_encrypted = resource.Body("encrypted", type=format.BoolStr)
def _action(self, session, body):
"""Preform volume actions given the message body."""
# NOTE: This is using Volume.base_path instead of self.base_path
# as both Volume and VolumeDetail instances can be acted on, but
# the URL used is sans any additional /detail/ part.
url = utils.urljoin(Volume.base_path, self.id, 'action')
headers = {'Accept': ''}
return session.post(url, json=body, headers=headers)
def extend(self, session, size):
"""Extend a volume size."""
body = {'os-extend': {'new_size': size}}
self._action(session, body)
VolumeDetail = Volume

22
openstack/block_storage/v3/_proxy.py
View File

@ -147,11 +147,9 @@ class Proxy(_base_proxy.BaseBlockStorageProxy):
def volumes(self, details=True, **query):
"""Retrieve a generator of volumes
:param bool details: When set to ``False``
:class:`~openstack.block_storage.v3.volume.Volume` objects
will be returned. The default, ``True``, will cause
:class:`~openstack.block_storage.v3.volume.VolumeDetail`
objects to be returned.
:param bool details: When set to ``False`` no extended attributes
will be returned. The default, ``True``, will cause objects with
additional attributes to be returned.
:param kwargs query: Optional query parameters to be sent to limit
the volumes being returned. Available parameters include:
@ -162,8 +160,8 @@ class Proxy(_base_proxy.BaseBlockStorageProxy):
:returns: A generator of volume objects.
"""
volume = _volume.VolumeDetail if details else _volume.Volume
return self._list(volume, **query)
base_path = '/volumes/detail' if details else None
return self._list(_volume.Volume, base_path=base_path, **query)
def create_volume(self, **attrs):
"""Create a new volume from attributes
@ -215,10 +213,8 @@ class Proxy(_base_proxy.BaseBlockStorageProxy):
"""Retrieve a generator of backups
:param bool details: When set to ``False``
:class:`~openstack.block_storage.v3.backup.Backup` objects
will be returned. The default, ``True``, will cause
:class:`~openstack.block_storage.v3.backup.BackupDetail`
objects to be returned.
no additional details will be returned. The default, ``True``,
will cause objects with additional attributes to be returned.
:param dict query: Optional query parameters to be sent to limit the
resources being returned:
@ -239,8 +235,8 @@ class Proxy(_base_proxy.BaseBlockStorageProxy):
raise exceptions.SDKException(
'Object-store service is required for block-store backups'
)
backup = _backup.BackupDetail if details else _backup.Backup
return self._list(backup, **query)
base_path = '/backups/detail' if details else None
return self._list(_backup.Backup, base_path=base_path, **query)
def get_backup(self, backup):
"""Get a backup

9
openstack/block_storage/v3/backup.py
View File

@ -90,11 +90,4 @@ class Backup(resource.Resource):
return self
class BackupDetail(Backup):
"""Volume Backup with Details"""
base_path = "/backups/detail"
# capabilities
allow_list = True
#: Properties
BackupDetail = Backup

43
openstack/block_storage/v3/volume.py
View File

@ -75,32 +75,6 @@ class Volume(resource.Resource):
#: The timestamp of this volume creation.
created_at = resource.Body("created_at")
def _action(self, session, body):
"""Preform volume actions given the message body."""
# NOTE: This is using Volume.base_path instead of self.base_path
# as both Volume and VolumeDetail instances can be acted on, but
# the URL used is sans any additional /detail/ part.
url = utils.urljoin(Volume.base_path, self.id, 'action')
headers = {'Accept': ''}
return session.post(url, json=body, headers=headers)
def extend(self, session, size):
"""Extend a volume size."""
body = {'os-extend': {'new_size': size}}
self._action(session, body)
class VolumeDetail(Volume):
base_path = "/volumes/detail"
# capabilities
allow_fetch = False
allow_create = False
allow_delete = False
allow_commit = False
allow_list = True
#: The volume's current back-end.
host = resource.Body("os-vol-host-attr:host")
#: The project ID associated with current back-end.
@ -123,3 +97,20 @@ class VolumeDetail(Volume):
#: ``True`` if this volume is encrypted, ``False`` if not.
#: *Type: bool*
is_encrypted = resource.Body("encrypted", type=format.BoolStr)
def _action(self, session, body):
"""Preform volume actions given the message body."""
# NOTE: This is using Volume.base_path instead of self.base_path
# as both Volume and VolumeDetail instances can be acted on, but
# the URL used is sans any additional /detail/ part.
url = utils.urljoin(Volume.base_path, self.id, 'action')
headers = {'Accept': ''}
return session.post(url, json=body, headers=headers)
def extend(self, session, size):
"""Extend a volume size."""
body = {'os-extend': {'new_size': size}}
self._action(session, body)
VolumeDetail = Volume

34
openstack/tests/unit/block_storage/v2/test_backup.py
View File

@ -10,7 +10,6 @@
# License for the specific language governing permissions and limitations
# under the License.
import copy
import mock
from keystoneauth1 import adapter
@ -38,12 +37,6 @@ BACKUP = {
"has_dependent_backups": False
}
DETAILS = {
}
BACKUP_DETAIL = copy.copy(BACKUP)
BACKUP_DETAIL.update(DETAILS)
class TestBackup(base.TestCase):
@ -92,30 +85,3 @@ class TestBackup(base.TestCase):
self.assertEqual(BACKUP["size"], sot.size)
self.assertEqual(BACKUP["has_dependent_backups"],
sot.has_dependent_backups)
class TestBackupDetail(base.TestCase):
def test_basic(self):
sot = backup.BackupDetail(BACKUP_DETAIL)
self.assertIsInstance(sot, backup.Backup)
self.assertEqual("/backups/detail", sot.base_path)
def test_create(self):
sot = backup.Backup(**BACKUP_DETAIL)
self.assertEqual(BACKUP_DETAIL["id"], sot.id)
self.assertEqual(BACKUP_DETAIL["name"], sot.name)
self.assertEqual(BACKUP_DETAIL["status"], sot.status)
self.assertEqual(BACKUP_DETAIL["container"], sot.container)
self.assertEqual(BACKUP_DETAIL["availability_zone"],
sot.availability_zone)
self.assertEqual(BACKUP_DETAIL["created_at"], sot.created_at)
self.assertEqual(BACKUP_DETAIL["updated_at"], sot.updated_at)
self.assertEqual(BACKUP_DETAIL["description"], sot.description)
self.assertEqual(BACKUP_DETAIL["fail_reason"], sot.fail_reason)
self.assertEqual(BACKUP_DETAIL["volume_id"], sot.volume_id)
self.assertEqual(BACKUP_DETAIL["object_count"], sot.object_count)
self.assertEqual(BACKUP_DETAIL["is_incremental"], sot.is_incremental)
self.assertEqual(BACKUP_DETAIL["size"], sot.size)
self.assertEqual(BACKUP_DETAIL["has_dependent_backups"],
sot.has_dependent_backups)

10
openstack/tests/unit/block_storage/v2/test_proxy.py
View File

@ -70,9 +70,10 @@ class TestVolumeProxy(test_proxy_base.TestProxyBase):
self.verify_get(self.proxy.get_volume, volume.Volume)
def test_volumes_detailed(self):
self.verify_list(self.proxy.volumes, volume.VolumeDetail,
self.verify_list(self.proxy.volumes, volume.Volume,
method_kwargs={"details": True, "query": 1},
expected_kwargs={"query": 1})
expected_kwargs={"query": 1,
"base_path": "/volumes/detail"})
def test_volumes_not_detailed(self):
self.verify_list(self.proxy.volumes, volume.Volume,
@ -101,9 +102,10 @@ class TestVolumeProxy(test_proxy_base.TestProxyBase):
# NOTE: mock has_service
self.proxy._connection = mock.Mock()
self.proxy._connection.has_service = mock.Mock(return_value=True)
self.verify_list(self.proxy.backups, backup.BackupDetail,
self.verify_list(self.proxy.backups, backup.Backup,
method_kwargs={"details": True, "query": 1},
expected_kwargs={"query": 1})
expected_kwargs={"query": 1,
"base_path": "/backups/detail"})
def test_backups_not_detailed(self):
# NOTE: mock has_service

61
openstack/tests/unit/block_storage/v2/test_volume.py
View File

@ -10,7 +10,6 @@
# License for the specific language governing permissions and limitations
# under the License.
import copy
import mock
from openstack.tests.unit import base
@ -42,10 +41,7 @@ VOLUME = {
"metadata": {},
"volume_image_metadata": IMAGE_METADATA,
"id": FAKE_ID,
"size": 10
}
DETAILS = {
"size": 10,
"os-vol-host-attr:host": "127.0.0.1",
"os-vol-tenant-attr:tenant_id": "some tenant",
"os-vol-mig-status-attr:migstat": "done",
@ -55,12 +51,9 @@ DETAILS = {
"consistencygroup_id": "123asf-asdf123",
"os-volume-replication:driver_data": "ahasadfasdfasdfasdfsdf",
"snapshot_id": "93c2e2aa-7744-4fd6-a31a-80c4726b08d7",
"encrypted": "false",
"encrypted": "false"
}
VOLUME_DETAIL = copy.copy(VOLUME)
VOLUME_DETAIL.update(DETAILS)
class TestVolume(base.TestCase):
@ -108,6 +101,23 @@ class TestVolume(base.TestCase):
sot.volume_image_metadata)
self.assertEqual(VOLUME["size"], sot.size)
self.assertEqual(VOLUME["imageRef"], sot.image_id)
self.assertEqual(VOLUME["os-vol-host-attr:host"], sot.host)
self.assertEqual(VOLUME["os-vol-tenant-attr:tenant_id"],
sot.project_id)
self.assertEqual(VOLUME["os-vol-mig-status-attr:migstat"],
sot.migration_status)
self.assertEqual(VOLUME["os-vol-mig-status-attr:name_id"],
sot.migration_id)
self.assertEqual(VOLUME["replication_status"],
sot.replication_status)
self.assertEqual(
VOLUME["os-volume-replication:extended_status"],
sot.extended_replication_status)
self.assertEqual(VOLUME["consistencygroup_id"],
sot.consistency_group_id)
self.assertEqual(VOLUME["os-volume-replication:driver_data"],
sot.replication_driver_data)
self.assertFalse(sot.is_encrypted)
def test_extend(self):
sot = volume.Volume(**VOLUME)
@ -118,36 +128,3 @@ class TestVolume(base.TestCase):
body = {"os-extend": {"new_size": "20"}}
headers = {'Accept': ''}
self.sess.post.assert_called_with(url, json=body, headers=headers)
class TestVolumeDetail(base.TestCase):
def test_basic(self):
sot = volume.VolumeDetail(VOLUME_DETAIL)
self.assertIsInstance(sot, volume.Volume)
self.assertEqual("/volumes/detail", sot.base_path)
self.assertFalse(sot.allow_fetch)
self.assertFalse(sot.allow_commit)
self.assertFalse(sot.allow_create)
self.assertFalse(sot.allow_delete)
self.assertTrue(sot.allow_list)
def test_create(self):
sot = volume.VolumeDetail(**VOLUME_DETAIL)
self.assertEqual(VOLUME_DETAIL["os-vol-host-attr:host"], sot.host)
self.assertEqual(VOLUME_DETAIL["os-vol-tenant-attr:tenant_id"],
sot.project_id)
self.assertEqual(VOLUME_DETAIL["os-vol-mig-status-attr:migstat"],
sot.migration_status)
self.assertEqual(VOLUME_DETAIL["os-vol-mig-status-attr:name_id"],
sot.migration_id)
self.assertEqual(VOLUME_DETAIL["replication_status"],
sot.replication_status)
self.assertEqual(
VOLUME_DETAIL["os-volume-replication:extended_status"],
sot.extended_replication_status)
self.assertEqual(VOLUME_DETAIL["consistencygroup_id"],
sot.consistency_group_id)
self.assertEqual(VOLUME_DETAIL["os-volume-replication:driver_data"],
sot.replication_driver_data)
self.assertFalse(sot.is_encrypted)

34
openstack/tests/unit/block_storage/v3/test_backup.py
View File

@ -10,7 +10,6 @@
# License for the specific language governing permissions and limitations
# under the License.
import copy
import mock
from keystoneauth1 import adapter
@ -38,12 +37,6 @@ BACKUP = {
"has_dependent_backups": False
}
DETAILS = {
}
BACKUP_DETAIL = copy.copy(BACKUP)
BACKUP_DETAIL.update(DETAILS)
class TestBackup(base.TestCase):
@ -92,30 +85,3 @@ class TestBackup(base.TestCase):
self.assertEqual(BACKUP["size"], sot.size)
self.assertEqual(BACKUP["has_dependent_backups"],
sot.has_dependent_backups)
class TestBackupDetail(base.TestCase):
def test_basic(self):
sot = backup.BackupDetail(BACKUP_DETAIL)
self.assertIsInstance(sot, backup.Backup)
self.assertEqual("/backups/detail", sot.base_path)
def test_create(self):
sot = backup.Backup(**BACKUP_DETAIL)
self.assertEqual(BACKUP_DETAIL["id"], sot.id)
self.assertEqual(BACKUP_DETAIL["name"], sot.name)
self.assertEqual(BACKUP_DETAIL["status"], sot.status)
self.assertEqual(BACKUP_DETAIL["container"], sot.container)
self.assertEqual(BACKUP_DETAIL["availability_zone"],
sot.availability_zone)
self.assertEqual(BACKUP_DETAIL["created_at"], sot.created_at)
self.assertEqual(BACKUP_DETAIL["updated_at"], sot.updated_at)
self.assertEqual(BACKUP_DETAIL["description"], sot.description)
self.assertEqual(BACKUP_DETAIL["fail_reason"], sot.fail_reason)
self.assertEqual(BACKUP_DETAIL["volume_id"], sot.volume_id)
self.assertEqual(BACKUP_DETAIL["object_count"], sot.object_count)
self.assertEqual(BACKUP_DETAIL["is_incremental"], sot.is_incremental)
self.assertEqual(BACKUP_DETAIL["size"], sot.size)
self.assertEqual(BACKUP_DETAIL["has_dependent_backups"],
sot.has_dependent_backups)

10
openstack/tests/unit/block_storage/v3/test_proxy.py
View File

@ -70,9 +70,10 @@ class TestVolumeProxy(test_proxy_base.TestProxyBase):
self.verify_get(self.proxy.get_volume, volume.Volume)
def test_volumes_detailed(self):
self.verify_list(self.proxy.volumes, volume.VolumeDetail,
self.verify_list(self.proxy.volumes, volume.Volume,
method_kwargs={"details": True, "query": 1},
expected_kwargs={"query": 1})
expected_kwargs={"query": 1,
"base_path": "/volumes/detail"})
def test_volumes_not_detailed(self):
self.verify_list(self.proxy.volumes, volume.Volume,
@ -101,9 +102,10 @@ class TestVolumeProxy(test_proxy_base.TestProxyBase):
# NOTE: mock has_service
self.proxy._connection = mock.Mock()
self.proxy._connection.has_service = mock.Mock(return_value=True)
self.verify_list(self.proxy.backups, backup.BackupDetail,
self.verify_list(self.proxy.backups, backup.Backup,
method_kwargs={"details": True, "query": 1},
expected_kwargs={"query": 1})
expected_kwargs={"query": 1,
"base_path": "/backups/detail"})
def test_backups_not_detailed(self):
# NOTE: mock has_service

61
openstack/tests/unit/block_storage/v3/test_volume.py
View File

@ -10,7 +10,6 @@
# License for the specific language governing permissions and limitations
# under the License.
import copy
import mock
from openstack.tests.unit import base
@ -42,10 +41,7 @@ VOLUME = {
"metadata": {},
"volume_image_metadata": IMAGE_METADATA,
"id": FAKE_ID,
"size": 10
}
DETAILS = {
"size": 10,
"os-vol-host-attr:host": "127.0.0.1",
"os-vol-tenant-attr:tenant_id": "some tenant",
"os-vol-mig-status-attr:migstat": "done",
@ -55,12 +51,9 @@ DETAILS = {
"consistencygroup_id": "123asf-asdf123",
"os-volume-replication:driver_data": "ahasadfasdfasdfasdfsdf",
"snapshot_id": "93c2e2aa-7744-4fd6-a31a-80c4726b08d7",
"encrypted": "false",
"encrypted": "false"
}
VOLUME_DETAIL = copy.copy(VOLUME)
VOLUME_DETAIL.update(DETAILS)
class TestVolume(base.TestCase):
@ -108,6 +101,23 @@ class TestVolume(base.TestCase):
sot.volume_image_metadata)
self.assertEqual(VOLUME["size"], sot.size)
self.assertEqual(VOLUME["imageRef"], sot.image_id)
self.assertEqual(VOLUME["os-vol-host-attr:host"], sot.host)
self.assertEqual(VOLUME["os-vol-tenant-attr:tenant_id"],
sot.project_id)
self.assertEqual(VOLUME["os-vol-mig-status-attr:migstat"],
sot.migration_status)
self.assertEqual(VOLUME["os-vol-mig-status-attr:name_id"],
sot.migration_id)
self.assertEqual(VOLUME["replication_status"],
sot.replication_status)
self.assertEqual(
VOLUME["os-volume-replication:extended_status"],
sot.extended_replication_status)
self.assertEqual(VOLUME["consistencygroup_id"],
sot.consistency_group_id)
self.assertEqual(VOLUME["os-volume-replication:driver_data"],
sot.replication_driver_data)
self.assertFalse(sot.is_encrypted)
def test_extend(self):
sot = volume.Volume(**VOLUME)
@ -118,36 +128,3 @@ class TestVolume(base.TestCase):
body = {"os-extend": {"new_size": "20"}}
headers = {'Accept': ''}
self.sess.post.assert_called_with(url, json=body, headers=headers)
class TestVolumeDetail(base.TestCase):
def test_basic(self):
sot = volume.VolumeDetail(VOLUME_DETAIL)
self.assertIsInstance(sot, volume.Volume)
self.assertEqual("/volumes/detail", sot.base_path)
self.assertFalse(sot.allow_fetch)
self.assertFalse(sot.allow_commit)
self.assertFalse(sot.allow_create)
self.assertFalse(sot.allow_delete)
self.assertTrue(sot.allow_list)
def test_create(self):
sot = volume.VolumeDetail(**VOLUME_DETAIL)
self.assertEqual(VOLUME_DETAIL["os-vol-host-attr:host"], sot.host)
self.assertEqual(VOLUME_DETAIL["os-vol-tenant-attr:tenant_id"],
sot.project_id)
self.assertEqual(VOLUME_DETAIL["os-vol-mig-status-attr:migstat"],
sot.migration_status)
self.assertEqual(VOLUME_DETAIL["os-vol-mig-status-attr:name_id"],
sot.migration_id)
self.assertEqual(VOLUME_DETAIL["replication_status"],
sot.replication_status)
self.assertEqual(
VOLUME_DETAIL["os-volume-replication:extended_status"],
sot.extended_replication_status)
self.assertEqual(VOLUME_DETAIL["consistencygroup_id"],
sot.consistency_group_id)
self.assertEqual(VOLUME_DETAIL["os-volume-replication:driver_data"],
sot.replication_driver_data)
self.assertFalse(sot.is_encrypted)

6
releasenotes/notes/remove-block-store-details-classes-158ab1f46655320a.yaml Normal file
View File

@ -0,0 +1,6 @@
---
deprecations:
- |
Requesting volumes or backups with details from block_storage will return
objects of classes Volume and Backup correspondingly, instead
of VolumeDetail and BackupDetail.