From ddd8d84cef2f70e694a29c59873a6bcbd2404298 Mon Sep 17 00:00:00 2001
From: Artem Goncharov <artem.goncharov@gmail.com>
Date: Sat, 9 Mar 2019 16:50:55 +0100
Subject: [PATCH] Deprecate ServerDetails class

We have recently introduced possibility to alter base_path for
operations on the resource and shifted some of the services to use it.
For compute.servers(details) we still use an inherited class with
modified base_path. Remove this class and instead alter the base_path
for the list servers operation. This also has an advantage, that we get
a list of servers with details and are immediately able to execute
operations on the instance without any conversion or re-fetch by id.

Change-Id: I4c4649ee1390676f3ccc6bf19e4b11b1964b5aaa
---
 openstack/compute/v2/_proxy.py                        | 10 ++++------
 openstack/compute/v2/server.py                        | 10 +---------
 openstack/tests/unit/compute/v2/test_proxy.py         |  5 +++--
 openstack/tests/unit/compute/v2/test_server.py        | 11 -----------
 ...emove-serverdetails-resource-f66cb278b224627d.yaml |  5 +++++
 5 files changed, 13 insertions(+), 28 deletions(-)
 create mode 100644 releasenotes/notes/remove-serverdetails-resource-f66cb278b224627d.yaml

diff --git a/openstack/compute/v2/_proxy.py b/openstack/compute/v2/_proxy.py
index 10727dd2c..215aad203 100644
--- a/openstack/compute/v2/_proxy.py
+++ b/openstack/compute/v2/_proxy.py
@@ -484,10 +484,8 @@ class Proxy(proxy.Proxy):
         """Retrieve a generator of servers
 
         :param bool details: When set to ``False``
-                    :class:`~openstack.compute.v2.server.Server` instances
-                    will be returned. The default, ``True``, will cause
-                    :class:`~openstack.compute.v2.server.ServerDetail`
-                    instances to be returned.
+            instances with only basic data will be returned. The default,
+            ``True``, will cause instances with full data to be returned.
         :param kwargs query: Optional query parameters to be sent to limit
             the servers being returned.  Available parameters include:
 
@@ -521,8 +519,8 @@ class Proxy(proxy.Proxy):
         """
         if all_projects:
             query['all_projects'] = True
-        srv = _server.ServerDetail if details else _server.Server
-        return self._list(srv, **query)
+        base_path = '/servers/detail' if details else None
+        return self._list(_server.Server, base_path=base_path, **query)
 
     def update_server(self, server, **attrs):
         """Update a server
diff --git a/openstack/compute/v2/server.py b/openstack/compute/v2/server.py
index 32eeec53f..1ed715876 100644
--- a/openstack/compute/v2/server.py
+++ b/openstack/compute/v2/server.py
@@ -440,12 +440,4 @@ class Server(resource.Resource, metadata.MetadataMixin, resource.TagMixin):
             session, {'os-migrateLive': body}, microversion=microversion)
 
 
-class ServerDetail(Server):
-    base_path = '/servers/detail'
-
-    # capabilities
-    allow_create = False
-    allow_fetch = False
-    allow_commit = False
-    allow_delete = False
-    allow_list = True
+ServerDetail = Server
diff --git a/openstack/tests/unit/compute/v2/test_proxy.py b/openstack/tests/unit/compute/v2/test_proxy.py
index 6b0cac5be..b9b8694f3 100644
--- a/openstack/tests/unit/compute/v2/test_proxy.py
+++ b/openstack/tests/unit/compute/v2/test_proxy.py
@@ -214,10 +214,11 @@ class TestComputeProxy(test_proxy_base.TestProxyBase):
         self.verify_get(self.proxy.get_server, server.Server)
 
     def test_servers_detailed(self):
-        self.verify_list(self.proxy.servers, server.ServerDetail,
+        self.verify_list(self.proxy.servers, server.Server,
                          method_kwargs={"details": True,
                                         "changes_since": 1, "image": 2},
-                         expected_kwargs={"changes_since": 1, "image": 2})
+                         expected_kwargs={"changes_since": 1, "image": 2,
+                                          "base_path": "/servers/detail"})
 
     def test_servers_not_detailed(self):
         self.verify_list(self.proxy.servers, server.Server,
diff --git a/openstack/tests/unit/compute/v2/test_server.py b/openstack/tests/unit/compute/v2/test_server.py
index b95b0ede0..f43934420 100644
--- a/openstack/tests/unit/compute/v2/test_server.py
+++ b/openstack/tests/unit/compute/v2/test_server.py
@@ -149,17 +149,6 @@ class TestServer(base.TestCase):
                          sot.scheduler_hints)
         self.assertEqual(EXAMPLE['OS-EXT-SRV-ATTR:user_data'], sot.user_data)
 
-    def test_detail(self):
-        sot = server.ServerDetail()
-        self.assertEqual('server', sot.resource_key)
-        self.assertEqual('servers', sot.resources_key)
-        self.assertEqual('/servers/detail', sot.base_path)
-        self.assertFalse(sot.allow_create)
-        self.assertFalse(sot.allow_fetch)
-        self.assertFalse(sot.allow_commit)
-        self.assertFalse(sot.allow_delete)
-        self.assertTrue(sot.allow_list)
-
     def test__prepare_server(self):
         zone = 1
         data = 2
diff --git a/releasenotes/notes/remove-serverdetails-resource-f66cb278b224627d.yaml b/releasenotes/notes/remove-serverdetails-resource-f66cb278b224627d.yaml
new file mode 100644
index 000000000..4f50e5587
--- /dev/null
+++ b/releasenotes/notes/remove-serverdetails-resource-f66cb278b224627d.yaml
@@ -0,0 +1,5 @@
+---
+deprecations:
+  - |
+    Listing servers with details `servers(details=True)` will return
+    instances of the Server class instead of ServerDetails.