Filter snapshots data on the DB side

It filters snapshots data on the DB side to improve performance.
Some tests are removed from test_snapshots.py because their function
implemented on the DB side.

Partial-Implements: blueprint data-filtering-on-the-db-side
Change-Id: I1c3e47a4f3780de54d7a5920f463e26cc8b7bcc2

diff --git a/cinder/db/api.py b/cinder/db/api.py
index c82689bc54e8f1da1fff2cf4bc649d88132346d6..45a63b318c0823caab4e455181998ff7a6981ff6 100644 (file)
--- a/cinder/db/api.py
+++ b/cinder/db/api.py
@@ -285,14 +285,14 @@ def snapshot_get(context, snapshot_id):
     return IMPL.snapshot_get(context, snapshot_id)
 
 
-def snapshot_get_all(context):
+def snapshot_get_all(context, filters=None):
     """Get all snapshots."""
-    return IMPL.snapshot_get_all(context)
+    return IMPL.snapshot_get_all(context, filters)
 
 
-def snapshot_get_all_by_project(context, project_id):
+def snapshot_get_all_by_project(context, project_id, filters=None):
     """Get all snapshots belonging to a project."""
-    return IMPL.snapshot_get_all_by_project(context, project_id)
+    return IMPL.snapshot_get_all_by_project(context, project_id, filters)
 
 
 def snapshot_get_by_host(context, host, filters=None):

diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py
index b0536c889b56f82fffc9f3f9ad51388c641adf1c..7dc908013b286d9724a354474c2bcc914f059493 100644 (file)
--- a/cinder/db/sqlalchemy/api.py
+++ b/cinder/db/sqlalchemy/api.py
@@ -1974,10 +1974,22 @@ def snapshot_get(context, snapshot_id):
 
 
 @require_admin_context
-def snapshot_get_all(context):
-    return model_query(context, models.Snapshot).\
-        options(joinedload('snapshot_metadata')).\
-        all()
+def snapshot_get_all(context, filters=None):
+    # Ensure that the filter value exists on the model
+    if filters:
+        for key in filters.keys():
+            try:
+                getattr(models.Snapshot, key)
+            except AttributeError:
+                LOG.debug("'%s' filter key is not valid.", key)
+                return []
+
+    query = model_query(context, models.Snapshot)
+
+    if filters:
+        query = query.filter_by(**filters)
+
+    return query.options(joinedload('snapshot_metadata')).all()
 
 
 @require_context
@@ -2012,12 +2024,15 @@ def snapshot_get_all_for_cgsnapshot(context, cgsnapshot_id):
 
 
 @require_context
-def snapshot_get_all_by_project(context, project_id):
+def snapshot_get_all_by_project(context, project_id, filters=None):
     authorize_project_context(context, project_id)
-    return model_query(context, models.Snapshot).\
-        filter_by(project_id=project_id).\
-        options(joinedload('snapshot_metadata')).\
-        all()
+    query = model_query(context, models.Snapshot)
+
+    if filters:
+        query = query.filter_by(**filters)
+
+    return query.filter_by(project_id=project_id).\
+        options(joinedload('snapshot_metadata')).all()
 
 
 @require_context

diff --git a/cinder/objects/snapshot.py b/cinder/objects/snapshot.py
index d266a756a7dedebbb41d1f39e861c3683ccbe03e..8c561358c9a81b7899239664263f586ee226d3d5 100644 (file)
--- a/cinder/objects/snapshot.py
+++ b/cinder/objects/snapshot.py
@@ -211,8 +211,8 @@ class SnapshotList(base.ObjectListBase, base.CinderObject):
     }
 
     @base.remotable_classmethod
-    def get_all(cls, context):
-        snapshots = db.snapshot_get_all(context)
+    def get_all(cls, context, search_opts):
+        snapshots = db.snapshot_get_all(context, search_opts)
         return base.obj_make_list(context, cls(), objects.Snapshot,
                                   snapshots,
                                   expected_attrs=['metadata'])
@@ -224,8 +224,9 @@ class SnapshotList(base.ObjectListBase, base.CinderObject):
                                   snapshots, expected_attrs=['metadata'])
 
     @base.remotable_classmethod
-    def get_all_by_project(cls, context, project_id):
-        snapshots = db.snapshot_get_all_by_project(context, project_id)
+    def get_all_by_project(cls, context, project_id, search_opts):
+        snapshots = db.snapshot_get_all_by_project(context, project_id,
+                                                   search_opts)
         return base.obj_make_list(context, cls(context), objects.Snapshot,
                                   snapshots, expected_attrs=['metadata'])
 

diff --git a/cinder/tests/unit/api/v1/stubs.py b/cinder/tests/unit/api/v1/stubs.py
index 764bfeec1b4ceb40bd37c68f81d834734d7f1ef5..2f1d40bb69bea7c1b332ca6f8423b6078a3ed03b 100644 (file)
--- a/cinder/tests/unit/api/v1/stubs.py
+++ b/cinder/tests/unit/api/v1/stubs.py
@@ -118,13 +118,13 @@ def stub_snapshot(id, **kwargs):
     return snapshot
 
 
-def stub_snapshot_get_all(self):
+def stub_snapshot_get_all(self, search_opts=None):
     return [stub_snapshot(100, project_id='fake'),
             stub_snapshot(101, project_id='superfake'),
             stub_snapshot(102, project_id='superduperfake')]
 
 
-def stub_snapshot_get_all_by_project(self, context):
+def stub_snapshot_get_all_by_project(self, context, search_opts=None):
     return [stub_snapshot(1)]
 
 

diff --git a/cinder/tests/unit/api/v1/test_snapshots.py b/cinder/tests/unit/api/v1/test_snapshots.py
index 3d498412d5aea4d2e50d37a62ccc752572bc1b76..fd4b7cc71eaad1402f312ebdcbab14afb3d9b86f 100644 (file)
--- a/cinder/tests/unit/api/v1/test_snapshots.py
+++ b/cinder/tests/unit/api/v1/test_snapshots.py
@@ -311,95 +311,6 @@ class SnapshotApiTest(test.TestCase):
         resp_snapshot = resp_snapshots.pop()
         self.assertEqual(resp_snapshot['id'], UUID)
 
-    @mock.patch('cinder.db.snapshot_metadata_get', return_value=dict())
-    def test_snapshot_list_by_status(self, snapshot_metadata_get):
-        def stub_snapshot_get_all_by_project(context, project_id):
-            return [
-                stubs.stub_snapshot(1, display_name='backup1',
-                                    status='available'),
-                stubs.stub_snapshot(2, display_name='backup2',
-                                    status='available'),
-                stubs.stub_snapshot(3, display_name='backup3',
-                                    status='creating'),
-            ]
-        self.stubs.Set(db, 'snapshot_get_all_by_project',
-                       stub_snapshot_get_all_by_project)
-
-        # no status filter
-        req = fakes.HTTPRequest.blank('/v1/snapshots')
-        resp = self.controller.index(req)
-        self.assertEqual(len(resp['snapshots']), 3)
-        # single match
-        req = fakes.HTTPRequest.blank('/v1/snapshots?status=creating')
-        resp = self.controller.index(req)
-        self.assertEqual(len(resp['snapshots']), 1)
-        self.assertEqual(resp['snapshots'][0]['status'], 'creating')
-        # multiple match
-        req = fakes.HTTPRequest.blank('/v1/snapshots?status=available')
-        resp = self.controller.index(req)
-        self.assertEqual(len(resp['snapshots']), 2)
-        for snapshot in resp['snapshots']:
-            self.assertEqual(snapshot['status'], 'available')
-        # no match
-        req = fakes.HTTPRequest.blank('/v1/snapshots?status=error')
-        resp = self.controller.index(req)
-        self.assertEqual(len(resp['snapshots']), 0)
-
-    @mock.patch('cinder.db.snapshot_metadata_get', return_value=dict())
-    def test_snapshot_list_by_volume(self, snapshot_metadata_get):
-        def stub_snapshot_get_all_by_project(context, project_id):
-            return [
-                stubs.stub_snapshot(1, volume_id='vol1', status='creating'),
-                stubs.stub_snapshot(2, volume_id='vol1', status='available'),
-                stubs.stub_snapshot(3, volume_id='vol2', status='available'),
-            ]
-        self.stubs.Set(db, 'snapshot_get_all_by_project',
-                       stub_snapshot_get_all_by_project)
-
-        # single match
-        req = fakes.HTTPRequest.blank('/v1/snapshots?volume_id=vol2')
-        resp = self.controller.index(req)
-        self.assertEqual(len(resp['snapshots']), 1)
-        self.assertEqual(resp['snapshots'][0]['volume_id'], 'vol2')
-        # multiple match
-        req = fakes.HTTPRequest.blank('/v1/snapshots?volume_id=vol1')
-        resp = self.controller.index(req)
-        self.assertEqual(len(resp['snapshots']), 2)
-        for snapshot in resp['snapshots']:
-            self.assertEqual(snapshot['volume_id'], 'vol1')
-        # multiple filters
-        req = fakes.HTTPRequest.blank('/v1/snapshots?volume_id=vol1'
-                                      '&status=available')
-        resp = self.controller.index(req)
-        self.assertEqual(len(resp['snapshots']), 1)
-        self.assertEqual(resp['snapshots'][0]['volume_id'], 'vol1')
-        self.assertEqual(resp['snapshots'][0]['status'], 'available')
-
-    @mock.patch('cinder.db.snapshot_metadata_get', return_value=dict())
-    def test_snapshot_list_by_name(self, snapshot_metadata_get):
-        def stub_snapshot_get_all_by_project(context, project_id):
-            return [
-                stubs.stub_snapshot(1, display_name='backup1'),
-                stubs.stub_snapshot(2, display_name='backup2'),
-                stubs.stub_snapshot(3, display_name='backup3'),
-            ]
-        self.stubs.Set(db, 'snapshot_get_all_by_project',
-                       stub_snapshot_get_all_by_project)
-
-        # no display_name filter
-        req = fakes.HTTPRequest.blank('/v1/snapshots')
-        resp = self.controller.index(req)
-        self.assertEqual(len(resp['snapshots']), 3)
-        # filter by one name
-        req = fakes.HTTPRequest.blank('/v1/snapshots?display_name=backup2')
-        resp = self.controller.index(req)
-        self.assertEqual(len(resp['snapshots']), 1)
-        self.assertEqual(resp['snapshots'][0]['display_name'], 'backup2')
-        # filter no match
-        req = fakes.HTTPRequest.blank('/v1/snapshots?display_name=backup4')
-        resp = self.controller.index(req)
-        self.assertEqual(len(resp['snapshots']), 0)
-
     @mock.patch('cinder.db.snapshot_metadata_get', return_value=dict())
     def test_admin_list_snapshots_limited_to_project(self,
                                                      snapshot_metadata_get):
@@ -414,7 +325,8 @@ class SnapshotApiTest(test.TestCase):
     def test_list_snapshots_with_limit_and_offset(self,
                                                   snapshot_metadata_get):
         def list_snapshots_with_limit_and_offset(is_admin):
-            def stub_snapshot_get_all_by_project(context, project_id):
+            def stub_snapshot_get_all_by_project(context, project_id,
+                                                 search_opts):
                 return [
                     stubs.stub_snapshot(1, display_name='backup1'),
                     stubs.stub_snapshot(2, display_name='backup2'),

diff --git a/cinder/tests/unit/api/v2/stubs.py b/cinder/tests/unit/api/v2/stubs.py
index ae5a690d99b739b1dacbf2158982d2229b76d6e1..3841e94b87be9bf150d3bb66a1e65db8d70c7f9e 100644 (file)
--- a/cinder/tests/unit/api/v2/stubs.py
+++ b/cinder/tests/unit/api/v2/stubs.py
@@ -153,13 +153,13 @@ def stub_snapshot(id, **kwargs):
     return snapshot
 
 
-def stub_snapshot_get_all(self):
+def stub_snapshot_get_all(self, search_opts=None):
     return [stub_snapshot(100, project_id='fake'),
             stub_snapshot(101, project_id='superfake'),
             stub_snapshot(102, project_id='superduperfake')]
 
 
-def stub_snapshot_get_all_by_project(self, context):
+def stub_snapshot_get_all_by_project(self, context, search_opts=None):
     return [stub_snapshot(1)]
 
 

diff --git a/cinder/tests/unit/api/v2/test_snapshots.py b/cinder/tests/unit/api/v2/test_snapshots.py
index 82ff94f05d8f4a276d2ea8ec77af56a2028fc20d..1a7e4f8d38c21118cec9efbdf64a45b04b1be663 100644 (file)
--- a/cinder/tests/unit/api/v2/test_snapshots.py
+++ b/cinder/tests/unit/api/v2/test_snapshots.py
@@ -323,95 +323,6 @@ class SnapshotApiTest(test.TestCase):
         resp_snapshot = resp_snapshots.pop()
         self.assertEqual(resp_snapshot['id'], UUID)
 
-    @mock.patch('cinder.db.snapshot_metadata_get', return_value=dict())
-    def test_snapshot_list_by_status(self, snapshot_metadata_get):
-        def stub_snapshot_get_all_by_project(context, project_id):
-            return [
-                stubs.stub_snapshot(1, display_name='backup1',
-                                    status='available'),
-                stubs.stub_snapshot(2, display_name='backup2',
-                                    status='available'),
-                stubs.stub_snapshot(3, display_name='backup3',
-                                    status='creating'),
-            ]
-        self.stubs.Set(db, 'snapshot_get_all_by_project',
-                       stub_snapshot_get_all_by_project)
-
-        # no status filter
-        req = fakes.HTTPRequest.blank('/v2/snapshots')
-        resp = self.controller.index(req)
-        self.assertEqual(len(resp['snapshots']), 3)
-        # single match
-        req = fakes.HTTPRequest.blank('/v2/snapshots?status=creating')
-        resp = self.controller.index(req)
-        self.assertEqual(len(resp['snapshots']), 1)
-        self.assertEqual(resp['snapshots'][0]['status'], 'creating')
-        # multiple match
-        req = fakes.HTTPRequest.blank('/v2/snapshots?status=available')
-        resp = self.controller.index(req)
-        self.assertEqual(len(resp['snapshots']), 2)
-        for snapshot in resp['snapshots']:
-            self.assertEqual(snapshot['status'], 'available')
-        # no match
-        req = fakes.HTTPRequest.blank('/v2/snapshots?status=error')
-        resp = self.controller.index(req)
-        self.assertEqual(len(resp['snapshots']), 0)
-
-    @mock.patch('cinder.db.snapshot_metadata_get', return_value={})
-    def test_snapshot_list_by_volume(self, snapshot_metadata_get):
-        def stub_snapshot_get_all_by_project(context, project_id):
-            return [
-                stubs.stub_snapshot(1, volume_id='vol1', status='creating'),
-                stubs.stub_snapshot(2, volume_id='vol1', status='available'),
-                stubs.stub_snapshot(3, volume_id='vol2', status='available'),
-            ]
-        self.stubs.Set(db, 'snapshot_get_all_by_project',
-                       stub_snapshot_get_all_by_project)
-
-        # single match
-        req = fakes.HTTPRequest.blank('/v2/snapshots?volume_id=vol2')
-        resp = self.controller.index(req)
-        self.assertEqual(len(resp['snapshots']), 1)
-        self.assertEqual(resp['snapshots'][0]['volume_id'], 'vol2')
-        # multiple match
-        req = fakes.HTTPRequest.blank('/v2/snapshots?volume_id=vol1')
-        resp = self.controller.index(req)
-        self.assertEqual(len(resp['snapshots']), 2)
-        for snapshot in resp['snapshots']:
-            self.assertEqual(snapshot['volume_id'], 'vol1')
-        # multiple filters
-        req = fakes.HTTPRequest.blank('/v2/snapshots?volume_id=vol1'
-                                      '&status=available')
-        resp = self.controller.index(req)
-        self.assertEqual(len(resp['snapshots']), 1)
-        self.assertEqual(resp['snapshots'][0]['volume_id'], 'vol1')
-        self.assertEqual(resp['snapshots'][0]['status'], 'available')
-
-    @mock.patch('cinder.db.snapshot_metadata_get', return_value={})
-    def test_snapshot_list_by_name(self, snapshot_metadata_get):
-        def stub_snapshot_get_all_by_project(context, project_id):
-            return [
-                stubs.stub_snapshot(1, display_name='backup1'),
-                stubs.stub_snapshot(2, display_name='backup2'),
-                stubs.stub_snapshot(3, display_name='backup3'),
-            ]
-        self.stubs.Set(db, 'snapshot_get_all_by_project',
-                       stub_snapshot_get_all_by_project)
-
-        # no name filter
-        req = fakes.HTTPRequest.blank('/v2/snapshots')
-        resp = self.controller.index(req)
-        self.assertEqual(len(resp['snapshots']), 3)
-        # filter by one name
-        req = fakes.HTTPRequest.blank('/v2/snapshots?name=backup2')
-        resp = self.controller.index(req)
-        self.assertEqual(len(resp['snapshots']), 1)
-        self.assertEqual(resp['snapshots'][0]['name'], 'backup2')
-        # filter no match
-        req = fakes.HTTPRequest.blank('/v2/snapshots?name=backup4')
-        resp = self.controller.index(req)
-        self.assertEqual(len(resp['snapshots']), 0)
-
     @mock.patch('cinder.db.snapshot_metadata_get', return_value=dict())
     def test_admin_list_snapshots_limited_to_project(self,
                                                      snapshot_metadata_get):
@@ -426,7 +337,8 @@ class SnapshotApiTest(test.TestCase):
     def test_list_snapshots_with_limit_and_offset(self,
                                                   snapshot_metadata_get):
         def list_snapshots_with_limit_and_offset(is_admin):
-            def stub_snapshot_get_all_by_project(context, project_id):
+            def stub_snapshot_get_all_by_project(context, project_id,
+                                                 search_opts):
                 return [
                     stubs.stub_snapshot(1, display_name='backup1'),
                     stubs.stub_snapshot(2, display_name='backup2'),

diff --git a/cinder/tests/unit/objects/test_snapshot.py b/cinder/tests/unit/objects/test_snapshot.py
index 5b928f8db4a57b92405268b89efaf749fbe9231c..6e824d138e377e7e8cb228652121845d476a363f 100644 (file)
--- a/cinder/tests/unit/objects/test_snapshot.py
+++ b/cinder/tests/unit/objects/test_snapshot.py
@@ -146,9 +146,12 @@ class TestSnapshotList(test_objects.BaseObjectsTestCase):
         fake_volume_obj = fake_volume.fake_volume_obj(self.context)
         volume_get_by_id.return_value = fake_volume_obj
 
-        snapshots = objects.SnapshotList.get_all(self.context)
+        search_opts = mock.sentinel.search_opts
+        snapshots = objects.SnapshotList.get_all(
+            self.context, search_opts)
         self.assertEqual(1, len(snapshots))
         TestSnapshot._compare(self, fake_snapshot, snapshots[0])
+        snapshot_get_all.assert_called_once_with(self.context, search_opts)
 
     @mock.patch('cinder.db.snapshot_metadata_get', return_value={})
     @mock.patch('cinder.objects.Volume.get_by_id')
@@ -173,10 +176,14 @@ class TestSnapshotList(test_objects.BaseObjectsTestCase):
         fake_volume_obj = fake_volume.fake_volume_obj(self.context)
         volume_get_by_id.return_value = fake_volume_obj
 
+        search_opts = mock.sentinel.search_opts
         snapshots = objects.SnapshotList.get_all_by_project(
-            self.context, self.project_id)
+            self.context, self.project_id, search_opts)
         self.assertEqual(1, len(snapshots))
         TestSnapshot._compare(self, fake_snapshot, snapshots[0])
+        get_all_by_project.assert_called_once_with(self.context,
+                                                   self.project_id,
+                                                   search_opts)
 
     @mock.patch('cinder.db.snapshot_metadata_get', return_value={})
     @mock.patch('cinder.objects.volume.Volume.get_by_id')

diff --git a/cinder/tests/unit/test_db_api.py b/cinder/tests/unit/test_db_api.py
index 74cb105b7e57d117c830c500bce8749686cf2fac..1688759cda3e66f30d92baa8f9d49821faa0ced7 100644 (file)
--- a/cinder/tests/unit/test_db_api.py
+++ b/cinder/tests/unit/test_db_api.py
@@ -954,11 +954,72 @@ class DBAPISnapshotTestCase(BaseTest):
         actual = db.snapshot_data_get_for_project(self.ctxt, 'project1')
         self.assertEqual(actual, (1, 42))
 
-    def test_snapshot_get_all(self):
+    def test_snapshot_get_all_by_filter(self):
         db.volume_create(self.ctxt, {'id': 1})
-        snapshot = db.snapshot_create(self.ctxt, {'id': 1, 'volume_id': 1})
-        self._assertEqualListsOfObjects([snapshot],
-                                        db.snapshot_get_all(self.ctxt),
+        db.volume_create(self.ctxt, {'id': 2})
+        snapshot1 = db.snapshot_create(self.ctxt, {'id': 1, 'volume_id': 1,
+                                                   'display_name': 'one',
+                                                   'status': 'available'})
+        snapshot2 = db.snapshot_create(self.ctxt, {'id': 2, 'volume_id': 1,
+                                                   'display_name': 'two',
+                                                   'status': 'creating'})
+        snapshot3 = db.snapshot_create(self.ctxt, {'id': 3, 'volume_id': 2,
+                                                   'display_name': 'three',
+                                                   'status': 'available'})
+        # no filter
+        filters = {}
+        snapshots = db.snapshot_get_all(self.ctxt, filters=filters)
+        self.assertEqual(3, len(snapshots))
+        # single match
+        filters = {'display_name': 'two'}
+        self._assertEqualListsOfObjects([snapshot2],
+                                        db.snapshot_get_all(
+                                            self.ctxt,
+                                            filters),
+                                        ignored_keys=['metadata', 'volume'])
+        filters = {'volume_id': 2}
+        self._assertEqualListsOfObjects([snapshot3],
+                                        db.snapshot_get_all(
+                                            self.ctxt,
+                                            filters),
+                                        ignored_keys=['metadata', 'volume'])
+        # filter no match
+        filters = {'volume_id': 5}
+        self._assertEqualListsOfObjects([],
+                                        db.snapshot_get_all(
+                                            self.ctxt,
+                                            filters),
+                                        ignored_keys=['metadata', 'volume'])
+        filters = {'status': 'error'}
+        self._assertEqualListsOfObjects([],
+                                        db.snapshot_get_all(
+                                            self.ctxt,
+                                            filters),
+                                        ignored_keys=['metadata', 'volume'])
+        # multiple match
+        filters = {'volume_id': 1}
+        self._assertEqualListsOfObjects([snapshot1, snapshot2],
+                                        db.snapshot_get_all(
+                                            self.ctxt,
+                                            filters),
+                                        ignored_keys=['metadata', 'volume'])
+        filters = {'status': 'available'}
+        self._assertEqualListsOfObjects([snapshot1, snapshot3],
+                                        db.snapshot_get_all(
+                                            self.ctxt,
+                                            filters),
+                                        ignored_keys=['metadata', 'volume'])
+        filters = {'volume_id': 1, 'status': 'available'}
+        self._assertEqualListsOfObjects([snapshot1],
+                                        db.snapshot_get_all(
+                                            self.ctxt,
+                                            filters),
+                                        ignored_keys=['metadata', 'volume'])
+        filters = {'fake_key': 'fake'}
+        self._assertEqualListsOfObjects([],
+                                        db.snapshot_get_all(
+                                            self.ctxt,
+                                            filters),
                                         ignored_keys=['metadata', 'volume'])
 
     def test_snapshot_get_by_host(self):

diff --git a/cinder/volume/api.py b/cinder/volume/api.py
index 21431c613e2cb0a4c3f2ecb3de7c53e12e4ac22d..11abff87e5c1010e72aef0549263e97b65c1b57b 100644 (file)
--- a/cinder/volume/api.py
+++ b/cinder/volume/api.py
@@ -507,23 +507,12 @@ class API(base.Base):
         if (context.is_admin and 'all_tenants' in search_opts):
             # Need to remove all_tenants to pass the filtering below.
             del search_opts['all_tenants']
-            snapshots = objects.SnapshotList.get_all(context)
+            snapshots = objects.SnapshotList.get_all(context,
+                                                     search_opts)
         else:
             snapshots = objects.SnapshotList.get_all_by_project(
-                context, context.project_id)
+                context, context.project_id, search_opts)
 
-        if search_opts:
-            LOG.debug("Searching by: %s", search_opts)
-
-            results = []
-            not_found = object()
-            for snapshot in snapshots:
-                for opt, value in search_opts.items():
-                    if snapshot.get(opt, not_found) != value:
-                        break
-                else:
-                    results.append(snapshot)
-            snapshots.objects = results
         LOG.info(_LI("Get all snaphsots completed successfully."))
         return snapshots