[7.10] Allow '_shards.skipped' to be absent on scroll responses

Co-authored-by: Seth Michael Larson <seth.larson@elastic.co>
elastic · Nov 20, 2020 · d8a9ccd · d8a9ccd
1 parent 231a013
commit d8a9ccd
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 22 deletions.
diff --git a/elasticsearch/_async/helpers.py b/elasticsearch/_async/helpers.py
@@ -336,24 +336,28 @@ async def async_scan(
             for hit in resp["hits"]["hits"]:
                 yield hit
 
+            # Default to 0 if the value isn't included in the response
+            shards_successful = resp["_shards"].get("successful", 0)
+            shards_skipped = resp["_shards"].get("skipped", 0)
+            shards_total = resp["_shards"].get("total", 0)
+
             # check if we have any errors
-            if (resp["_shards"]["successful"] + resp["_shards"]["skipped"]) < resp[
-                "_shards"
-            ]["total"]:
+            if (shards_successful + shards_skipped) < shards_total:
+                shards_message = "Scroll request has only succeeded on %d (+%d skipped) shards out of %d."
                 logger.warning(
-                    "Scroll request has only succeeded on %d (+%d skipped) shards out of %d.",
-                    resp["_shards"]["successful"],
-                    resp["_shards"]["skipped"],
-                    resp["_shards"]["total"],
+                    shards_message,
+                    shards_successful,
+                    shards_skipped,
+                    shards_total,
                 )
                 if raise_on_error:
                     raise ScanError(
                         scroll_id,
-                        "Scroll request has only succeeded on %d (+%d skipped) shards out of %d."
+                        shards_message
                         % (
-                            resp["_shards"]["successful"],
-                            resp["_shards"]["skipped"],
-                            resp["_shards"]["total"],
+                            shards_successful,
+                            shards_skipped,
+                            shards_total,
                         ),
                     )
             resp = await client.scroll(

diff --git a/elasticsearch/helpers/actions.py b/elasticsearch/helpers/actions.py
@@ -531,24 +531,28 @@ def scan(
             for hit in resp["hits"]["hits"]:
                 yield hit
 
+            # Default to 0 if the value isn't included in the response
+            shards_successful = resp["_shards"].get("successful", 0)
+            shards_skipped = resp["_shards"].get("skipped", 0)
+            shards_total = resp["_shards"].get("total", 0)
+
             # check if we have any errors
-            if (resp["_shards"]["successful"] + resp["_shards"]["skipped"]) < resp[
-                "_shards"
-            ]["total"]:
+            if (shards_successful + shards_skipped) < shards_total:
+                shards_message = "Scroll request has only succeeded on %d (+%d skipped) shards out of %d."
                 logger.warning(
-                    "Scroll request has only succeeded on %d (+%d skipped) shards out of %d.",
-                    resp["_shards"]["successful"],
-                    resp["_shards"]["skipped"],
-                    resp["_shards"]["total"],
+                    shards_message,
+                    shards_successful,
+                    shards_skipped,
+                    shards_total,
                 )
                 if raise_on_error:
                     raise ScanError(
                         scroll_id,
-                        "Scroll request has only succeeded on %d (+%d skipped) shards out of %d."
+                        shards_message
                         % (
-                            resp["_shards"]["successful"],
-                            resp["_shards"]["skipped"],
-                            resp["_shards"]["total"],
+                            shards_successful,
+                            shards_skipped,
+                            shards_total,
                         ),
                     )
             resp = client.scroll(

diff --git a/test_elasticsearch/test_server/test_helpers.py b/test_elasticsearch/test_server/test_helpers.py
@@ -508,6 +508,33 @@ def test_clear_scroll(self):
             )
             spy.assert_not_called()
 
+    def test_shards_no_skipped_field(self):
+        with patch.object(self, "client") as client_mock:
+            client_mock.search.return_value = {
+                "_scroll_id": "dummy_id",
+                "_shards": {"successful": 5, "total": 5},
+                "hits": {"hits": [{"search_data": 1}]},
+            }
+            client_mock.scroll.side_effect = [
+                {
+                    "_scroll_id": "dummy_id",
+                    "_shards": {"successful": 5, "total": 5},
+                    "hits": {"hits": [{"scroll_data": 42}]},
+                },
+                {
+                    "_scroll_id": "dummy_id",
+                    "_shards": {"successful": 5, "total": 5},
+                    "hits": {"hits": []},
+                },
+            ]
+
+            data = list(
+                helpers.scan(
+                    self.client, index="test_index", size=2, raise_on_error=True
+                )
+            )
+            self.assertEqual(data, [{"search_data": 1}, {"scroll_data": 42}])
+
 
 class TestReindex(ElasticsearchTestCase):
     def setup_method(self, _):