Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Address review feedback:
Browse files Browse the repository at this point in the history
- Use fstrings in exception messages
- Ensure that a warning is logged but execution still proceeds when searches
  include a `pages` parameter
- Document that the `pages` parameter is deprecated
- Test that unknown operation types passed to the Query runner cause an
  exception to be raised
Mike Baamonde committed Oct 27, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent 79506e1 commit 313408f
Showing 3 changed files with 81 additions and 3 deletions.
2 changes: 1 addition & 1 deletion docs/track.rst
Original file line number Diff line number Diff line change
@@ -968,7 +968,7 @@ Properties
* ``body`` (mandatory): The query body.
* ``response-compression-enabled`` (optional, defaults to ``true``): Allows to disable HTTP compression of responses. As these responses are sometimes large and decompression may be a bottleneck on the client, it is possible to turn off response compression.
* ``detailed-results`` (optional, defaults to ``false``): Records more detailed meta-data about queries. As it analyzes the corresponding response in more detail, this might incur additional overhead which can skew measurement results. This flag is ineffective for scroll queries.
* ``pages`` (optional): Number of pages to retrieve. If this parameter is present, a scroll query will be executed. If you want to retrieve all result pages, use the value "all". See also the ``scroll-search`` operation type.
* ``pages`` (optional, deprecated): Number of pages to retrieve. If this parameter is present, a scroll query will be executed. If you want to retrieve all result pages, use the value "all". This parameter is deprecated and will be replaced with the ``scroll-search`` operation in a future release.
* ``results-per-page`` (optional): Number of documents to retrieve per page. This maps to the Search API's ``size`` parameter, and can be used for scroll and non-scroll searches. Defaults to ``10``

Example::
5 changes: 3 additions & 2 deletions esrally/driver/runner.py
Original file line number Diff line number Diff line change
@@ -103,7 +103,7 @@ def runner_for(operation_type):
try:
return __RUNNERS[operation_type]
except KeyError:
raise exceptions.RallyError("No runner available for operation type [%s]" % operation_type)
raise exceptions.RallyError(f"No runner available for operation-type: {operation_type}]")


def enable_assertions(enabled):
@@ -982,10 +982,11 @@ async def _scroll_query(es, params):
"Invoking a scroll search with the 'search' operation is deprecated "
"and will be removed in a future release. Use 'scroll-search' instead."
)
return await _scroll_query(es, params)
else:
return await _request_body_query(es, params)
else:
raise exceptions.RallyError("No runner available for operation type [%s]" % operation_type)
raise exceptions.RallyError(f"No runner available for operation-type: {operation_type}")

async def _raw_search(self, es, doc_type, index, body, params, headers=None):
components = []
77 changes: 77 additions & 0 deletions tests/driver/runner_test.py
Original file line number Diff line number Diff line change
@@ -2091,6 +2091,83 @@ async def test_query_runner_missing_operation_type(self, es):

self.assertEqual(0, es.call_count)

@mock.patch("elasticsearch.Elasticsearch")
@run_async
async def test_query_runner_search_with_pages_logs_warning_and_executes(self, es):
# page 1
search_response = {
"_scroll_id": "some-scroll-id",
"took": 4,
"timed_out": False,
"hits": {
"total": {"value": 2, "relation": "eq"},
"hits": [
{"title": "some-doc-1"},
{"title": "some-doc-2"},
],
},
}

es.transport.perform_request = mock.AsyncMock(return_value=io.StringIO(json.dumps(search_response)))
es.clear_scroll = mock.AsyncMock(return_value=io.StringIO('{"acknowledged": true}'))

query_runner = runner.Query()

params = {
"operation-type": "search",
"pages": 1,
"results-per-page": 100,
"index": "unittest",
"cache": True,
"body": {
"query": {
"match_all": {},
},
},
}

with mock.patch.object(query_runner.logger, "warning") as mocked_warning_logger:
results = await query_runner(es, params)
mocked_warning_logger.assert_has_calls(
[
mock.call(
"Invoking a scroll search with the 'search' operation is deprecated "
"and will be removed in a future release. Use 'scroll-search' instead."
)
]
)

self.assertEqual(1, results["weight"])
self.assertEqual(1, results["pages"])
self.assertEqual(2, results["hits"])
self.assertEqual("eq", results["hits_relation"])
self.assertEqual(4, results["took"])
self.assertEqual("pages", results["unit"])
self.assertFalse(results["timed_out"])
self.assertFalse("error-type" in results)

@mock.patch("elasticsearch.Elasticsearch")
@run_async
async def test_query_runner_fails_with_unknown_operation_type(self, es):
query_runner = runner.Query()

params = {
"operation-type": "unknown",
"index": "unittest",
"body": {
"query": {
"match_all": {},
},
},
}

with self.assertRaises(exceptions.RallyError) as ctx:
await query_runner(es, params)
self.assertEqual(
"No runner available for operation-type: unknown",
ctx.exception.args[0],
)


class PutPipelineRunnerTests(TestCase):
@mock.patch("elasticsearch.Elasticsearch")

0 comments on commit 313408f

Please sign in to comment.